From: iproctor Date: Fri, 10 Aug 2007 20:48:12 +0000 (+0000) Subject: Thrift: OCaml TSocket fix X-Git-Tag: 0.2.0~1274 X-Git-Url: https://source.supwisdom.com/gerrit/gitweb?a=commitdiff_plain;h=e470aa38f548f80b0d8f924b17bc7f8eb93b1421;p=common%2Fthrift.git Thrift: OCaml TSocket fix Summary: Now closes input channel on close. Also, transport exceptions are cleaner. Reviewed by: mcslee Test plan: Yes Revert plan: yes git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@665198 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/lib/ocaml/README b/lib/ocaml/README index 9f871fbd..b3043676 100644 --- a/lib/ocaml/README +++ b/lib/ocaml/README @@ -1,18 +1,34 @@ Library ------- -The library abstract classes, exceptions, and general use functions are mostly jammed in Thrift.ml (an exception being TServer). Implementations live in their own files. I'm on the fence about whether it should be done with objects or modules/functors. Right now they are objects. TBinaryProtocol and TSocket are implemented. TServer and TSimpleServer classes are there, but the fastest route to a binary protocol socket server is to use TServer.run_basic_server which uses OCaml's own server abstraction. To that end, there is TChannelTransport which is a transport class parametrized on input and output channels that does nothing but wrap up the input and output functions. +The library abstract classes, exceptions, and general use functions +are mostly jammed in Thrift.ml (an exception being +TServer). -A note on making the library: Running make should create native and bytecode libraries. +Generally, classes are used, however they are often put in their own +module along with other relevant types and functions. The classes +often called t, exceptions are called E. + +Implementations live in their own files. There is TBinaryProtocol, +TSocket, TThreadedServer, TSimpleServer, and TServerSocket. + +A note on making the library: Running make should create native, debug +code libraries, and a toplevel. Struct format ------------- -Structs are turned into classes. The fields are all option types and are initially None. Write is a method, but reading is done by a separate function (since there is no such thing as a static class). I'm still arguing with myself about whether structs should be put in their own modules along with this read function. +Structs are turned into classes. The fields are all option types and +are initially None. Write is a method, but reading is done by a +separate function (since there is no such thing as a static +class). The class type is t and is in a module with the name of the +struct. -enum format +enum format ----------- -Enums are put in their own module along with functions to_i and of_i which convert the ocaml types into ints. For example: +Enums are put in their own module along with +functions to_i and of_i which convert the ocaml types into ints. For +example: enum Numberz { @@ -26,7 +42,7 @@ enum Numberz ==> -module Numbers = +module Numberz = struct type t = | ONE @@ -51,18 +67,29 @@ type userid Int64.t exception format ---------------- -Exceptions are kind of ugly since the exception structs can't be thrown directly. They also have this exception type which has the name BLAHBLAH_exn. For example, for an exception Xception you get: +The same as structs except that the module also has an exception type +E of t that is raised/caught. -exception Xception_exn of xception +For example, with an exception Xception, +raise (Xception.E (new Xception.t)) +and +try + ... +with Xception.E e -> ... list format ----------- -Lists are turned into OCaml native lists +Lists are turned into OCaml native lists. Map/Set formats --------------- -These are both turned into Hashtbl.t's. +These are both turned into Hashtbl.t's. Set values are bool. Services -------- -The client is a class "client" parametrized on input and output protocols. The processor is a class parametrized on a handler. A handler is a class inheriting the iface abstract class. Unlike other implementations, client does not implement iface since iface functions must take option arguments so as to deal with the case where a client does not send all the arguments. +The client is a class "client" parametrized on input and output +protocols. The processor is a class parametrized on a handler. A +handler is a class inheriting the iface abstract class. Unlike other +implementations, client does not implement iface since iface functions +must take option arguments so as to deal with the case where a client +does not send all the arguments. diff --git a/lib/ocaml/src/Makefile b/lib/ocaml/src/Makefile index 723402b1..20b09862 100644 --- a/lib/ocaml/src/Makefile +++ b/lib/ocaml/src/Makefile @@ -2,6 +2,6 @@ SOURCES = Thrift.ml TBinaryProtocol.ml TSocket.ml TChannelTransport.ml TServer.m RESULT = thrift LIBS = unix threads THREADS = yes -all: native-code-library byte-code-library top +all: native-code-library debug-code-library top OCAMLMAKEFILE = ../OCamlMakefile include $(OCAMLMAKEFILE) diff --git a/lib/ocaml/src/TBinaryProtocol.ml b/lib/ocaml/src/TBinaryProtocol.ml index 748423f9..fa84e712 100644 --- a/lib/ocaml/src/TBinaryProtocol.ml +++ b/lib/ocaml/src/TBinaryProtocol.ml @@ -114,7 +114,7 @@ object (self) let ver = self#readI32 in if (ver land version_mask != version_1) then (print_int ver; - raise (P.TProtocolExn (P.BAD_VERSION, "Missing version identifier"))) + raise (P.E (P.BAD_VERSION, "Missing version identifier"))) else let s = self#readString in let mt = P.message_type_of_i (ver land 0xFF) in diff --git a/lib/ocaml/src/TChannelTransport.ml b/lib/ocaml/src/TChannelTransport.ml index 89ae352c..5407a8e8 100644 --- a/lib/ocaml/src/TChannelTransport.ml +++ b/lib/ocaml/src/TChannelTransport.ml @@ -3,14 +3,18 @@ module T = Transport class t (i,o) = object (self) + val mutable opened = true inherit Transport.t - method isOpen = true + method isOpen = opened method opn = () - method close = () + method close = close_in i; opened <- false method read buf off len = - try - really_input i buf off len; len - with _ -> T.raise_TTransportExn ("TChannelTransport: Could not read "^(string_of_int len)) T.UNKNOWN + if opened then + try + really_input i buf off len; len + with _ -> raise (T.E (T.UNKNOWN, ("TChannelTransport: Could not read "^(string_of_int len)))) + else + raise (T.E (T.NOT_OPEN, "TChannelTransport: Channel was closed")) method write buf off len = output o buf off len method flush = flush o end diff --git a/lib/ocaml/src/TServer.ml b/lib/ocaml/src/TServer.ml index a4dcc442..5fb8089b 100644 --- a/lib/ocaml/src/TServer.ml +++ b/lib/ocaml/src/TServer.ml @@ -17,8 +17,7 @@ let run_basic_server proc port = let trans = new TChannelTransport.t (inp,out) in let proto = new TBinaryProtocol.t (trans :> Transport.t) in try - while proc#process proto proto do () done; - () + while proc#process proto proto do () done; () with e -> ()) (Unix.ADDR_INET (Unix.inet_addr_of_string "127.0.0.1",port)) diff --git a/lib/ocaml/src/TServerSocket.ml b/lib/ocaml/src/TServerSocket.ml index ac98b087..9e797060 100644 --- a/lib/ocaml/src/TServerSocket.ml +++ b/lib/ocaml/src/TServerSocket.ml @@ -11,11 +11,12 @@ object Unix.listen s 256 method close = match sock with - Some s -> Unix.shutdown s Unix.SHUTDOWN_ALL; Unix.close s; sock <- None + Some s -> Unix.shutdown s Unix.SHUTDOWN_ALL; Unix.close s; + sock <- None | _ -> () method acceptImpl = match sock with Some s -> let (fd,_) = Unix.accept s in new TChannelTransport.t (Unix.in_channel_of_descr fd,Unix.out_channel_of_descr fd) - | _ -> Transport.raise_TTransportExn "ServerSocket: Not listening but tried to accept" Transport.NOT_OPEN + | _ -> raise (Transport.E (Transport.NOT_OPEN,"TServerSocket: Not listening but tried to accept")) end diff --git a/lib/ocaml/src/TSocket.ml b/lib/ocaml/src/TSocket.ml index c02f1eba..c74864a9 100644 --- a/lib/ocaml/src/TSocket.ml +++ b/lib/ocaml/src/TSocket.ml @@ -9,23 +9,28 @@ object (self) method isOpen = chans != None method opn = try - chans <- Some(Unix.open_connection (Unix.ADDR_INET ((Unix.inet_addr_of_string host),port))) + let addr = (let {Unix.h_addr_list=x} = Unix.gethostbyname host in x.(0)) in + chans <- Some(Unix.open_connection (Unix.ADDR_INET (addr,port))) with _ -> - T.raise_TTransportExn - ("Could not connect to "^host^":"^(string_of_int port)) - T.NOT_OPEN - method close = match chans with None -> () | Some(inc,_) -> (Unix.shutdown_connection inc; chans <- None) + raise (T.E (T.NOT_OPEN, ("TSocket: Could not connect to "^host^":"^(string_of_int port)))) + + method close = + match chans with + None -> () + | Some(inc,out) -> (Unix.shutdown_connection inc; + close_in inc; + chans <- None) method read buf off len = match chans with - None -> T.raise_TTransportExn "Socket not open" T.NOT_OPEN + None -> raise (T.E (T.NOT_OPEN, "TSocket: Socket not open")) | Some(i,o) -> try really_input i buf off len; len - with _ -> T.raise_TTransportExn ("TSocket: Could not read "^(string_of_int len)^" from "^host^":"^(string_of_int port)) T.UNKNOWN + with _ -> raise (T.E (T.UNKNOWN, ("TSocket: Could not read "^(string_of_int len)^" from "^host^":"^(string_of_int port)))) method write buf off len = match chans with - None -> T.raise_TTransportExn "Socket not open" T.NOT_OPEN + None -> raise (T.E (T.NOT_OPEN, "TSocket: Socket not open")) | Some(i,o) -> output o buf off len method flush = match chans with - None -> T.raise_TTransportExn "Socket not open" T.NOT_OPEN + None -> raise (T.E (T.NOT_OPEN, "TSocket: Socket not open")) | Some(i,o) -> flush o end diff --git a/lib/ocaml/src/Thrift.ml b/lib/ocaml/src/Thrift.ml index 8ff5fa9d..92c015b6 100644 --- a/lib/ocaml/src/Thrift.ml +++ b/lib/ocaml/src/Thrift.ml @@ -9,11 +9,6 @@ object method set_message s = message <- s end;; -exception TExn of t_exn;; - - - - module Transport = struct type exn_type = @@ -23,19 +18,7 @@ struct | TIMED_OUT | END_OF_FILE;; - class exn = - object - inherit t_exn - val mutable typ = UNKNOWN - method get_type = typ - method set_type t = typ <- t - end - exception TTransportExn of exn - let raise_TTransportExn message typ = - let e = new exn in - e#set_message message; - e#set_type typ; - raise (TTransportExn e) + exception E of exn_type * string class virtual t = object (self) @@ -49,10 +32,7 @@ struct while !got < len do ret := self#read buf (off+(!got)) (len - (!got)); if !ret <= 0 then - let e = new exn in - e#set_message "Cannot read. Remote side has closed."; - raise (TTransportExn e) - else (); + raise (E (UNKNOWN, "Cannot read. Remote side has closed.")); got := !got + !ret done; !got @@ -260,7 +240,7 @@ struct | SIZE_LIMIT | BAD_VERSION - exception TProtocolExn of exn_type * string;; + exception E of exn_type * string;; end;; @@ -280,7 +260,7 @@ struct end - +(* Ugly *) module Application_Exn = struct type typ= @@ -336,7 +316,7 @@ struct let read (iprot : Protocol.t) = let msg = ref "" in let typ = ref 0 in - iprot#readStructBegin; + ignore iprot#readStructBegin; (try while true do let (name,ft,id) =iprot#readFieldBegin in