From b6dcd2b595724fd9c3d575466db8394dc16be3ae Mon Sep 17 00:00:00 2001 From: Christopher Piro Date: Sat, 13 Oct 2007 01:11:46 +0000 Subject: [PATCH] [thrift] retool oop.erl, fix tBufferedTransportFactory.erl Summary: oop.erl used to assume that an undef or function_clause meant a method wasn't defined, but sometimes a method does exist and an undef happens while it's executing. Case in point, getTransport in tBufferedTransportFactory totally didn't work and instead of exiting, oop.erl fell back silently to tTransportFactory, so everywhere I thought I was talking to tBufferedTransport, I was talking directly to the tSocket. borkborkbork. Blame: all me baby Reviewed By: eletuchy Test Plan: channel server works Revert Plan: ok git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@665299 13f79535-47bb-0310-9956-ffa450edef68 --- lib/erl/src/oop.erl | 298 +++++++++++------- lib/erl/src/protocol/tBinaryProtocol.erl | 12 +- lib/erl/src/transport/tBufferedTransport.erl | 2 +- .../transport/tBufferedTransportFactory.erl | 2 +- lib/erl/src/transport/tSocket.erl | 6 +- lib/erl/src/transport/tTransport.erl | 12 +- 6 files changed, 196 insertions(+), 136 deletions(-) diff --git a/lib/erl/src/oop.erl b/lib/erl/src/oop.erl index 08c0e37d..20edb86d 100644 --- a/lib/erl/src/oop.erl +++ b/lib/erl/src/oop.erl @@ -4,143 +4,143 @@ %%% See accompanying file LICENSE or visit the Thrift site at: %%% http://developers.facebook.com/thrift/ +%%% +%%% dox: +%%% +%%% C++ <-> Erlang +%%% classes modules +%%% class b : public a a:super() -> b. +%%% + -module(oop). --export([get/2, set/3, call/2, call/3, inspect/1, start_new/2, is_object/1, class/1]). +-export([start_new/2, get/2, set/3, call/2, call/3, inspect/1, class/1, is_object/1]). +-export([call1/3]). %% only for thrift_oop_server ... don't use it -export([behaviour_info/1]). -include("thrift.hrl"). -include("oop.hrl"). +%% state for the call loop +-record(cstate, { + obj, %% the current object (on which we want to invoke MFA) + module, %% the current module we're considering + func, %% the method name (i.e. the function we're trying to invoke in Module) + args, %% the arguments, the first of which is Obj + tried, %% a (backwards) list of modules we've tried + first_obj %% the original object + }). + %%% %%% behavior definition %%% -behaviour_info(callbacks) -> - [ - {attr, 4}, - {super, 0} - ]; -behaviour_info(_) -> +behaviour_info(callbacks) -> + [ {attr, 4}, + {super, 0} + ]; +behaviour_info(_) -> undefined. -%% - --define(TRIED, lists:reverse([TryModule|TriedRev])). - -%% no super attr defined --define(NOSUPEROBJ, exit({missing_attr_super, {inspect(Obj), ?TRIED}})). - --define(NOMETHOD, exit({missing_method, {Method, inspect(Obj), tl(Args), ?TRIED}})). - --define(NOATTR, exit({missing_attr, {hd(tl(Args)), inspect(FirstObj), ?TRIED}})). - --define(NOATTR_SET, exit({missing_attr, {Field, inspect(Obj), ".." %% TODO: give a backtrace - }})). - +%%% +%%% public interface +%%% -%%% get(Obj, Field) -> term() -%%% looks up Field in Obj or its ancestor objects +%% TODO: voids take only ok as return? +start_new(none=Resv, _) -> + error_logger:format("can't instantiate ~p: class name is a reserved word", [Resv]), + error; +start_new(Class, Args) -> + {ok, Pid} = gen_server:start_link(thrift_oop_server, {Class, Args}, []), + Pid. +%% get(Obj, Field) -> term() +%% looks up Field in Obj or its ancestor objects get(Obj, Field) -> call(Obj, attr, [get, Field, get]). set(Obj, Field, Value) -> %% TODO: could be tail-recursive Module = ?CLASS(Obj), - try - Module:attr(Obj, set, Field, Value) - catch - error:Kind when Kind == undef; Kind == function_clause -> - case get_superobject(Obj) of + case apply_if_defined(Module, attr, [Obj, set, Field, Value]) of + {ok, V} -> V; + undef -> + case get_superobject(Obj) of { ok, Superobj } -> - Super1 = set(Superobj, Field, Value), - try - Module:attr(Obj, set, super, Super1) - catch %% TODO(cpiro): remove check - X -> exit({burnsauce, X}) - end; - none -> - ?NOATTR_SET + Superobj1 = set(Superobj, Field, Value), + Module:attr(Obj, set, super, Superobj1); + undef -> + error(missing_attr_set, Field, Obj) end end. -%%% C++ <-> Erlang -%%% classes modules -%%% class b : public a a:super() -> b. -%%% +%% +%% ** dynamic method dispatch ** +%% +%% calls Module:Func(*Args) if it exists +%% if not, Module <- Module:super() and try again recursively +%% +%% Module:attr(*Args) is handled specially: +%% Obj needs to be replaced with Obj's "superobject" +%% +call(Obj, Func) -> + call(Obj, Func, []). + +call(Obj, Func, ArgsProper) -> + %% error_logger:info_msg("call called: Obj=~p Func=~p ArgsProper=~p", [inspect(Obj), Func, ArgsProper]), + case call1(Obj, Func, ArgsProper) of + {ok, Value} -> Value; + {error, Kind, S1} -> error(Kind, S1) + end. -get_superobject(Obj) -> - try - {ok, (?CLASS(Obj)):attr(Obj, get, super, get)} - catch - error:Kind when Kind == undef; Kind == function_clause -> - none +call1(Obj, Func, ArgsProper) -> + S = #cstate{ + obj = Obj, + module = ?CLASS(Obj), + func = Func, + args = [Obj|ArgsProper], %% prepend This to args + tried = [], + first_obj = Obj + }, + call1(S). + +call1(S = #cstate{obj=Obj, module=Module, func=Func, args=Args}) -> + %% error_logger:info_msg("call1~n obj=~p~n MFA=~p, ~p, ~p", [inspect(Obj), Module, Func, Args]), + %% io:format("call ~p~n", [Module]), + case apply_if_defined(Module, Func, Args) of + {ok, Value} -> {ok, Value}; + undef -> call1_try_super(S) end. -is_object(Obj) when is_tuple(Obj) -> - try - (?CLASS(Obj)):super(), %% if it's an object its first element will be a class name, and it'll have super/0 - true - catch - error:Kind when Kind == undef; Kind == function_clause -> - false +call1_try_super(S = #cstate{func=attr, module=Module, tried=Tried}) -> + case Module:super() of + none -> {error, missing_attr, S}; + Superclass -> call1_try_super_attr(Superclass, S) end; -is_object(_) -> - false. - -call(Obj, Method, ArgsProper) -> - %% error_logger:info_msg("call called: Obj=~p Method=~p ArgsProper=~p", [inspect(Obj), Method, ArgsProper]), - Args = [Obj|ArgsProper], %% prepend This to args - TryModule = ?CLASS(Obj), - call_loop(Obj, Method, Args, TryModule, [], Obj). - -call(Obj, Method) -> - call(Obj, Method, []). - -call_loop(Obj, Method, Args, TryModule, TriedRev, FirstObj) -> - try - %% error_logger:info_msg("call_loop~n ~p~n ~p~n ~p~n ~p", [inspect(Obj), Method, Args, TryModule]), - apply(TryModule, Method, Args) - catch - error:Kind when Kind == undef; Kind == function_clause -> - case { TryModule:super(), Method } of - { none, attr } -> - ?NOATTR; - - { none, _ } -> - ?NOMETHOD; - - { Superclass, attr } -> - %% look for attrs in the "super object" - - case get_superobject(Obj) of - {ok, Superobj} when (TryModule == ?CLASS(Obj)) -> - %% replace This with Superobj - NewArgs = [Superobj|tl(Args)], - call_loop(Superobj, Method, NewArgs, - Superclass, [TryModule|TriedRev], FirstObj); - - {ok, _Superobj} -> % failed guard TODO(cpiro): removeme - exit(oh_noes); - - none -> ?NOSUPEROBJ - end; - - { SuperClass, _ } -> - call_loop(Obj, Method, Args, - SuperClass, [TryModule|TriedRev], FirstObj) - end +call1_try_super(S = #cstate{func=Func, module=Module, tried=Tried}) -> + case Module:super() of + none -> {error, missing_method, S}; + Superclass -> + S1 = S#cstate{ + module = Superclass, + tried = [Module|Tried] + }, + call1(S1) + end. + +call1_try_super_attr(Superclass, S = #cstate{obj=Obj, module=Module, args=Args, tried=Tried}) -> + %% look for attrs in the "super object" + case get_superobject(Obj) of + undef -> {error, missing_superobj, S}; + {ok, Superobj} when Module == ?CLASS(Obj) -> + %% replace This with Superobj + S1 = S#cstate{ + obj = Superobj, + args = [Superobj|tl(Args)], + module = Superclass, + tried = [Module|Tried] + }, + call1(S1) end. - -class(Obj) when is_tuple(Obj) -> - case is_object(Obj) of - true -> - ?CLASS(Obj); - false -> - none - end; -class(_) -> - none. %% careful: not robust against records beginning with a class name %% (note: we can't just guard with is_record(?CLASS(Obj), Obj) since we @@ -149,7 +149,7 @@ inspect(Obj) -> try case is_object(Obj) of true -> - DeepList = inspect_loop(Obj, "#<"), + DeepList = inspect1(Obj, "#<"), lists:flatten(DeepList); false -> thrift_utils:sformat("~p", [Obj]) @@ -162,22 +162,80 @@ inspect(Obj) -> %% _:E -> thrift_utils:sformat("~p", [Obj]) end. -inspect_loop(Obj, Str) -> +inspect1(Obj, Str) -> Class = ?CLASS(Obj), Inspect = Class:inspect(Obj), Current = atom_to_list(Class) ++ ": " ++ Inspect, case get_superobject(Obj) of - { ok, Superobj } -> - inspect_loop(Superobj, Str ++ Current ++ " | "); - none -> + {ok, Superobj} -> + inspect1(Superobj, Str ++ Current ++ " | "); + undef -> Str ++ Current ++ ">" end. -%% TODO: voids take only ok as return? -start_new(none=Resv, _) -> - error_logger:format("can't instantiate ~p: class name is a reserved word", [Resv]), - error; -start_new(Class, Args) -> - {ok, Pid} = gen_server:start_link(thrift_oop_server, {Class, Args}, []), - Pid. +%% class(Obj) -> atom() = Class +%% | none +class(Obj) when is_tuple(Obj) -> + %% if it's an object its first element will be a class name, and it'll have super/0 + case apply_if_defined(?CLASS(Obj), super, []) of + {ok, V} -> V; + undef -> none + end; +class(_) -> none. + +%% is the tuple/record an object? +%% is_object(Obj) = bool() +is_object(Obj) when is_tuple(Obj) -> + case class(Obj) of + none -> false; + _ -> true + end; +is_object(_) -> false. + +%%% +%%% private helpers +%%% + +%% apply_if_defined(MFA) -> {ok, apply(MFA)} +%% | undef +%% this could be worth some money +apply_if_defined(M, F, A) -> + apply_if_defined({M,F,A}). + +apply_if_defined({M,F,A} = MFA) -> + try + %% io:format("apply ~p ~p ~p~n", [M,F,A]), + {ok, apply(M, F, A)} + catch + error:Kind when Kind == undef; Kind == function_clause -> + case erlang:get_stacktrace() of + %% the first stack call should match MFA when `apply' fails because the function is undefined + %% they won't match if the function is currently running and an error happens in the middle + [MFA|_] -> undef; % trapped successfully + ST -> + io:format("DONIT THE EXIT THING ~p~n", [Kind]), + exit({Kind, ST}) % some unrelated error, re-exit + end + end. + +get_superobject(Obj) -> + apply_if_defined(?CLASS(Obj), attr, [Obj, get, super, get]). + +%%% +%%% errors +%%% + +tried(S = #cstate{module=Module, tried=Tried}) -> + lists:reverse([Module|Tried]). + +error(missing_superobj, S = #cstate{obj=Obj}) -> + exit({missing_superobj, {inspect(Obj), tried(S)}}); +error(missing_method, S = #cstate{obj=Obj, func=Func, args=Args}) -> + exit({missing_method, {Func, inspect(Obj), tl(Args), tried(S)}}); +error(missing_attr, S = #cstate{args=Args, first_obj=FirstObj}) -> + exit({missing_attr, {hd(tl(Args)), inspect(FirstObj), tried(S)}}). + +error(missing_attr_set, Field, Obj) -> + BT = "..", %% TODO: give a backtrace + exit({missing_attr, {Field, inspect(Obj), BT}}). diff --git a/lib/erl/src/protocol/tBinaryProtocol.erl b/lib/erl/src/protocol/tBinaryProtocol.erl index b7452984..9430e39e 100644 --- a/lib/erl/src/protocol/tBinaryProtocol.erl +++ b/lib/erl/src/protocol/tBinaryProtocol.erl @@ -113,28 +113,28 @@ writeBool(This, Bool) -> writeByte(This, Byte) -> Trans = oop:get(This, trans), - ?R1(Trans, write, binary_to_list(<>)). + ?R1(Trans, effectful_write, binary_to_list(<>)). writeI16(This, I16) -> Trans = oop:get(This, trans), - ?R1(Trans, write, binary_to_list(<>)). + ?R1(Trans, effectful_write, binary_to_list(<>)). writeI32(This, I32) -> Trans = oop:get(This, trans), - ?R1(Trans, write, binary_to_list(<>)). + ?R1(Trans, effectful_write, binary_to_list(<>)). writeI64(This, I64) -> Trans = oop:get(This, trans), - ?R1(Trans, write, binary_to_list(<>)). + ?R1(Trans, effectful_write, binary_to_list(<>)). writeDouble(This, Double) -> Trans = oop:get(This, trans), - ?R1(Trans, write, binary_to_list(<>)). + ?R1(Trans, effectful_write, binary_to_list(<>)). writeString(This, Str) -> Trans = oop:get(This, trans), ?L1(writeI32, length(Str)), - ?R1(Trans, write, Str). + ?R1(Trans, effectful_write, Str). % diff --git a/lib/erl/src/transport/tBufferedTransport.erl b/lib/erl/src/transport/tBufferedTransport.erl index 48d9fba3..3c0a0460 100644 --- a/lib/erl/src/transport/tBufferedTransport.erl +++ b/lib/erl/src/transport/tBufferedTransport.erl @@ -39,7 +39,7 @@ super() -> %%% inspect(This) -> string() inspect(This) -> - ?FORMAT_ATTR(transport) ++ + ?FORMAT_ATTR(transport) ++ ", " ++ ?FORMAT_ATTR(wbuf). %%% diff --git a/lib/erl/src/transport/tBufferedTransportFactory.erl b/lib/erl/src/transport/tBufferedTransportFactory.erl index 9746aba5..5bc5012e 100644 --- a/lib/erl/src/transport/tBufferedTransportFactory.erl +++ b/lib/erl/src/transport/tBufferedTransportFactory.erl @@ -51,4 +51,4 @@ new() -> %%% getTransport(_This, Trans) -> - gen_server:start_link(tBufferedTransport, {new, [Trans]}). + oop:start_new(tBufferedTransport, [Trans]). diff --git a/lib/erl/src/transport/tSocket.erl b/lib/erl/src/transport/tSocket.erl index c2bf920d..c438203e 100644 --- a/lib/erl/src/transport/tSocket.erl +++ b/lib/erl/src/transport/tSocket.erl @@ -19,7 +19,7 @@ -export([new/0, new/1, new/2, effectful_setHandle/2, effectful_open/1, - isOpen/1, write/2, read/2, effectful_close/1]). + isOpen/1, effectful_write/2, read/2, effectful_close/1]). %%% %%% define attributes @@ -87,7 +87,7 @@ effectful_open(This) -> isOpen(This) -> oop:get(This, handle) /= nil. -write(This, Str) -> +effectful_write(This, Str) -> Handle = oop:get(This, handle), Val = gen_tcp:send(Handle, Str), @@ -100,7 +100,7 @@ write(This, Str) -> {error, _} -> throw(tTransportException:new(?tTransportException_NOT_OPEN, "in write")); ok -> - ok + {ok, This} end. read(This, Sz) -> diff --git a/lib/erl/src/transport/tTransport.erl b/lib/erl/src/transport/tTransport.erl index 6ec115a6..a7293be6 100644 --- a/lib/erl/src/transport/tTransport.erl +++ b/lib/erl/src/transport/tTransport.erl @@ -50,12 +50,13 @@ new() -> %%% instance methods %%% +e() -> + exit('tTransport is abstract'). - -isOpen(_This) -> nil. -open(_This) -> nil. -close(_This) -> nil. -read(_This, _Sz) -> nil. +isOpen(_This) -> e(), nil. +open(_This) -> e(), nil. +close(_This) -> e(), nil. +read(_This, _Sz) -> e(), nil. readAll(This, Sz) -> readAll_loop(This, Sz, "", 0). @@ -80,6 +81,7 @@ readAll_loop(This, Sz, Buff, Have) -> end. effectful_write(This, _Buf) -> + e(), {nil, This}. effectful_flush(This) -> -- 2.17.1