From c054ac3d4df7fe8b2f58204e677f275bd8f0e60c Mon Sep 17 00:00:00 2001 From: Dan Gudmundsson Date: Wed, 30 Oct 2013 16:46:54 +0100 Subject: wx: Fix cleanup of event handlers Event handlers was not removed after objects/process had been delete/died, which causes memory leakage and that fun's was kept in the wx_server process. Code that might be purged and the server died. --- lib/wx/src/wxe_server.erl | 118 +++++++++++++++++++++++++++++++--------------- 1 file changed, 79 insertions(+), 39 deletions(-) (limited to 'lib/wx/src') diff --git a/lib/wx/src/wxe_server.erl b/lib/wx/src/wxe_server.erl index a8604c76b9..45184a3bc8 100644 --- a/lib/wx/src/wxe_server.erl +++ b/lib/wx/src/wxe_server.erl @@ -36,7 +36,7 @@ terminate/2, code_change/3]). -record(state, {port,cb_port,users,cleaners=[],cb,cb_cnt}). --record(user, {objects=[], events=[], evt_handler}). +-record(user, {events=[], evt_handler}). -record(event, {object, callback, cb_handler}). -define(APPLICATION, wxe). @@ -157,26 +157,40 @@ handle_cast(_Msg, State) -> handle_info(Cb = {_, _, '_wx_invoke_cb_'}, State) -> invoke_cb(Cb, State), {noreply, State}; -handle_info({wx_delete_cb, FunId}, State0 = #state{cb=CB}) when is_integer(FunId) -> - case get(FunId) of - undefined -> - {noreply, State0}; - Fun -> - erase(FunId), - {noreply, State0#state{cb=gb_trees:delete(Fun, CB)}} + +handle_info({wx_delete_cb, FunId}, State) + when is_integer(FunId) -> + {noreply, delete_fun(FunId, State)}; + +handle_info({wx_delete_cb, Id, EvtListener, Obj}, State = #state{users=Users}) -> + From = erase(EvtListener), + case gb_trees:lookup(From, Users) of + none -> + {noreply, delete_fun(Id, State)}; + {value, User0} -> + User = cleanup_evt_listener(User0, EvtListener, Obj), + {noreply, delete_fun(Id, State#state{users=gb_trees:update(From, User, Users)})} end; + handle_info({'DOWN',_,process,Pid,_}, State=#state{users=Users0,cleaners=Cs}) -> try User = gb_trees:get(Pid,Users0), Users = gb_trees:delete(Pid,Users0), Env = wx:get_env(), - Cleaner = spawn_link(fun() -> cleanup(Env,Pid,[User]) end), - {noreply, State#state{users=Users,cleaners=[Cleaner|Cs]}} + case User of + #user{events=[], evt_handler=undefined} -> %% No need to spawn + {noreply, State#state{users=Users,cleaners=Cs}}; + _ -> + Cleaner = spawn_link(fun() -> cleanup(Env,Pid,[User]) end), + {noreply, State#state{users=Users,cleaners=[Cleaner|Cs]}} + end catch _E:_R -> %% ?log("Error: ~p ~p", [_E,_R]), {noreply, State} end; -handle_info(Msg = {'_wxe_destroy_', Pid}, State) -> + +handle_info(Msg = {'_wxe_destroy_', Pid}, State) + when is_pid(Pid) -> case erlang:is_process_alive(Pid) of true -> Pid ! Msg, @@ -185,6 +199,7 @@ handle_info(Msg = {'_wxe_destroy_', Pid}, State) -> ok end, {noreply, State}; + handle_info(_Info, State) -> ?log("Unknown message ~p sent to ~p~n",[_Info, ?MODULE]), {noreply, State}. @@ -212,10 +227,10 @@ handle_connect(Object, EvData, From, State0 = #state{users=Users}) -> CBHandler = Handler0, Handler = Handler0; undefined when Callback =:= 0 -> - Handler = new_evt_listener(State0), + Handler = new_evt_listener(State0, From), CBHandler = Handler; _ -> - CBHandler = new_evt_listener(State0), + CBHandler = new_evt_listener(State0, From), Handler = Handler0 end, Evs = [#event{object=Object,callback=Callback, cb_handler=CBHandler}|Evs0], @@ -238,9 +253,9 @@ handle_connect(Object, EvData, From, State0 = #state{users=Users}) -> invoke_cb({{Ev=#wx{}, Ref=#wx_ref{}}, FunId,_}, _S) -> %% Event callbacks case get(FunId) of - Fun when is_function(Fun) -> + {Fun,_} when is_function(Fun) -> invoke_callback(fun() -> Fun(Ev, Ref), <<>> end); - Pid when is_pid(Pid) -> %% wx_object sync event + {Pid,_} when is_pid(Pid) -> %% wx_object sync event invoke_callback(Pid, Ev, Ref); Err -> ?log("Internal Error ~p~n",[Err]) @@ -248,7 +263,7 @@ invoke_cb({{Ev=#wx{}, Ref=#wx_ref{}}, FunId,_}, _S) -> invoke_cb({FunId, Args, _}, _S) when is_list(Args), is_integer(FunId) -> %% Overloaded functions case get(FunId) of - Fun when is_function(Fun) -> + {Fun,_} when is_function(Fun) -> invoke_callback(fun() -> Fun(Args) end); Err -> ?log("Internal Error ~p ~p ~p~n",[Err, FunId, Args]) @@ -311,10 +326,12 @@ get_wx_object_state(Pid) -> _ -> ignore end. -new_evt_listener(State) -> +new_evt_listener(State, Owner) -> #wx_env{port=Port} = wx:get_env(), _ = erlang:port_control(Port,98,<<>>), - get_result(State). + Listener = get_result(State), + put(Listener, Owner), + Listener. get_result(_State) -> receive @@ -326,43 +343,66 @@ get_result(_State) -> attach_fun(Fun, S = #state{cb=CB,cb_cnt=Next}) -> case gb_trees:lookup(Fun,CB) of {value, ID} -> + {Fun, N} = get(ID), + put(ID, {Fun,N+1}), {ID,S}; none -> - put(Next,Fun), + put(Next,{Fun, 1}), {Next,S#state{cb=gb_trees:insert(Fun,Next,CB),cb_cnt=Next+1}} end. +delete_fun(0, State) -> State; +delete_fun(FunId, State = #state{cb=CB}) -> + case get(FunId) of + undefined -> + State; + {Fun,N} when N < 2 -> + erase(FunId), + State#state{cb=gb_trees:delete(Fun, CB)}; + {Fun,N} -> + put(FunId, {Fun, N-1}), + State + end. + +cleanup_evt_listener(U=#user{events=Evs0,evt_handler=Handler}, EvtListener, Object) -> + {Evs, Del} = lists:foldl(fun(#event{object=Obj,cb_handler=CBH}, {Acc, Delete}) + when CBH =:= EvtListener, Obj =:= Object -> + {Acc, Delete}; + (Event = #event{cb_handler=CBH}, {Acc, _Delete}) + when CBH =:= EvtListener -> + {[Event|Acc], false}; + (Event, {Acc, Delete}) -> + {[Event|Acc], Delete} + end, {[], true}, Evs0), + Del andalso wxEvtHandler:destroy_evt_listener(EvtListener), + case Del andalso Handler =:= EvtListener of + true -> + U#user{events=Evs, evt_handler=undefined}; + false -> + U#user{events=Evs} + end. + handle_disconnect(Object, Evh, From, State0 = #state{users=Users0}) -> - User0 = #user{events=Evs0, evt_handler=PidH} = gb_trees:get(From, Users0), + #user{events=Evs0} = gb_trees:get(From, Users0), Fun = wxEvtHandler:get_callback(Evh), case find_handler(Evs0, Object, Fun) of - [] -> - {reply, false, State0}; + [] -> {reply, false, State0}; Handlers -> case disconnect(Object,Evh, Handlers) of - Ev = #event{callback=CB, cb_handler=Handler} -> - case is_function(CB) of - true -> wxEvtHandler:destroy_evt_listener(Handler); - false -> ignore - end, - User = case lists:delete(Ev,Evs0) of - [] when PidH =/= undefined -> - wxEvtHandler:destroy_evt_listener(PidH), - User0#user{events=[], evt_handler=undefined}; - Evs -> - User0#user{events=Evs} - end, - {reply, true, State0#state{users=gb_trees:update(From,User,Users0)}}; + #event{} -> + {reply, true, State0}; Result -> {reply, Result, State0} end end. disconnect(Object,Evh,[Ev=#event{cb_handler=Handler}|Evs]) -> - case wxEvtHandler:disconnect_impl(Handler,Object,Evh) of + try wxEvtHandler:disconnect_impl(Handler,Object,Evh) of true -> Ev; false -> disconnect(Object, Evh, Evs); Error -> Error + catch _:_ -> + false end; disconnect(_, _, []) -> false. @@ -393,9 +433,9 @@ cleanup(Env, _Pid, Data) -> gen_server:cast(Env#wx_env.sv, {cleaned, self()}), normal. -cleanup(#user{objects=_Os,events=Evs, evt_handler=Handler}) -> - lists:foreach(fun(#event{object=_O, callback=CB, cb_handler=CbH}) -> - %%catch wxEvtHandler:disconnect_impl(CbH,O), +cleanup(#user{events=Evs, evt_handler=Handler}) -> + lists:foreach(fun(#event{object=O, callback=CB, cb_handler=CbH}) -> + catch wxEvtHandler:disconnect_impl(CbH,O), case is_function(CB) of true -> wxEvtHandler:destroy_evt_listener(CbH); -- cgit v1.2.3