From 30283e3136c22444a2ecb1263a77654e6f1bd48d Mon Sep 17 00:00:00 2001 From: Dan Gudmundsson Date: Thu, 14 Jul 2011 11:36:50 +0200 Subject: Handle overloading callbacks the same as events Previously other process wx calls where allowed to execute in the callback code, but that lead to a deadlock if for example a dialog was created. --- lib/wx/c_src/wxePrintout.cpp | 49 +++++++++++++++++++++++++-------------- lib/wx/c_src/wxe_impl.cpp | 43 ++++++++++++++++------------------ lib/wx/c_src/wxe_impl.h | 3 +-- lib/wx/src/wxe_server.erl | 52 ++++++++++++++++++++---------------------- lib/wx/test/wx_class_SUITE.erl | 2 +- 5 files changed, 79 insertions(+), 70 deletions(-) diff --git a/lib/wx/c_src/wxePrintout.cpp b/lib/wx/c_src/wxePrintout.cpp index 1bcbc5f1a4..90959df379 100644 --- a/lib/wx/c_src/wxePrintout.cpp +++ b/lib/wx/c_src/wxePrintout.cpp @@ -38,7 +38,7 @@ rt.addAtom("_wx_invoke_cb_"); \ rt.addTupleCount(3); \ rt.send(); \ - handle_callback_batch(port); \ + handle_event_callback(port, memenv->owner); \ } #define INVOKE_CALLBACK(port, callback, class_str) \ @@ -68,9 +68,11 @@ bool wxEPrintout::OnBeginDocument(int startPage, int endPage) rt.addInt(startPage); rt.addInt(endPage); INVOKE_CALLBACK_END(port, 2); - if(((WxeApp *) wxTheApp)->cb_len > 0) { - char * bp = ((WxeApp *) wxTheApp)->cb_buff; - return *(int*) bp; + if(((WxeApp *) wxTheApp)->cb_buff) { + int res = * (int*) ((WxeApp *) wxTheApp)->cb_buff; + driver_free(((WxeApp *) wxTheApp)->cb_buff); + ((WxeApp *) wxTheApp)->cb_buff = NULL; + return res; } } return wxPrintout::OnBeginDocument(startPage,endPage); @@ -122,9 +124,11 @@ bool wxEPrintout::HasPage(int page) INVOKE_CALLBACK_INIT(port, hasPage, "wxPrintout"); rt.addInt(page); INVOKE_CALLBACK_END(port, 1); - if(((WxeApp *) wxTheApp)->cb_len > 0) { - char * bp = ((WxeApp *) wxTheApp)->cb_buff; - return *(int*) bp; + if(((WxeApp *) wxTheApp)->cb_buff) { + int res = * (int*) ((WxeApp *) wxTheApp)->cb_buff; + driver_free(((WxeApp *) wxTheApp)->cb_buff); + ((WxeApp *) wxTheApp)->cb_buff = NULL; + return res; } } return wxPrintout::HasPage(page); @@ -135,9 +139,11 @@ bool wxEPrintout::OnPrintPage(int page) INVOKE_CALLBACK_INIT(port, onPrintPage, "wxPrintout"); rt.addInt(page); INVOKE_CALLBACK_END(port, 1); - if(((WxeApp *) wxTheApp)->cb_len > 0) { - char * bp = ((WxeApp *) wxTheApp)->cb_buff; - return *(int*) bp; + if(((WxeApp *) wxTheApp)->cb_buff) { + int res = * (int*) ((WxeApp *) wxTheApp)->cb_buff; + driver_free(((WxeApp *) wxTheApp)->cb_buff); + ((WxeApp *) wxTheApp)->cb_buff = NULL; + return res; } return FALSE; } @@ -146,12 +152,14 @@ void wxEPrintout::GetPageInfo(int *minPage, int *maxPage, int *pageFrom, int *pa { if(getPageInfo) { INVOKE_CALLBACK(port, getPageInfo, "wxPrintout"); - if(((WxeApp *) wxTheApp)->cb_len > 0) { + if(((WxeApp *) wxTheApp)->cb_buff) { char * bp = ((WxeApp *) wxTheApp)->cb_buff; *minPage = *(int *) bp; bp += 4; *maxPage = *(int *) bp; bp += 4; *pageFrom = *(int *) bp; bp += 4; *pageTo = *(int *) bp; bp += 4; + driver_free(((WxeApp *) wxTheApp)->cb_buff); + ((WxeApp *) wxTheApp)->cb_buff = NULL; } } wxPrintout::GetPageInfo(minPage, maxPage, pageFrom, pageTo); @@ -166,9 +174,11 @@ wxString EwxListCtrl::OnGetItemText(long item, long col) const { rt.addInt(item); rt.addInt(col); INVOKE_CALLBACK_END(port, 2); - if(((WxeApp *) wxTheApp)->cb_len > 0) { + if(((WxeApp *) wxTheApp)->cb_buff) { char * bp = ((WxeApp *) wxTheApp)->cb_buff; wxString str = wxString(bp, wxConvUTF8); + driver_free(((WxeApp *) wxTheApp)->cb_buff); + ((WxeApp *) wxTheApp)->cb_buff = NULL; return str; } } @@ -182,8 +192,11 @@ wxListItemAttr* EwxListCtrl::OnGetItemAttr(long item) const { INVOKE_CALLBACK_END(port, 1); char * bp = ((WxeApp *) wxTheApp)->cb_buff; wxeMemEnv * memenv = ((WxeApp *) wxTheApp)->getMemEnv(port); - if(((WxeApp *) wxTheApp)->cb_len > 0) { - return (wxListItemAttr *) ((WxeApp *) wxTheApp)->getPtr(bp, memenv); + if(bp) { + wxListItemAttr * result = (wxListItemAttr *)((WxeApp *) wxTheApp)->getPtr(bp, memenv); + driver_free(((WxeApp *) wxTheApp)->cb_buff); + ((WxeApp *) wxTheApp)->cb_buff = NULL; + return result; } } return NULL; @@ -199,9 +212,11 @@ int EwxListCtrl::OnGetItemColumnImage(long item, long col) const { rt.addInt(item); rt.addInt(col); INVOKE_CALLBACK_END(port, 2); - if(((WxeApp *) wxTheApp)->cb_len > 0) { - char * bp = ((WxeApp *) wxTheApp)->cb_buff; - return *(int *) bp; + if(((WxeApp *) wxTheApp)->cb_buff) { + int res = * (int*) ((WxeApp *) wxTheApp)->cb_buff; + driver_free(((WxeApp *) wxTheApp)->cb_buff); + ((WxeApp *) wxTheApp)->cb_buff = NULL; + return res; } } return -1; diff --git a/lib/wx/c_src/wxe_impl.cpp b/lib/wx/c_src/wxe_impl.cpp index 897c2b7cca..95755978f1 100644 --- a/lib/wx/c_src/wxe_impl.cpp +++ b/lib/wx/c_src/wxe_impl.cpp @@ -270,6 +270,7 @@ bool WxeApp::OnInit() global_me = new wxeMemEnv(); wxe_batch = new wxList; wxe_batch_cb_saved = new wxList; + cb_buff = NULL; wxIdleEvent::SetMode(wxIDLE_PROCESS_SPECIFIED); @@ -330,24 +331,14 @@ void handle_event_callback(ErlDrvPort port, ErlDrvTermData process) driver_monitor_process(port, process, &monitor); // Should we be able to handle commands when recursing? probably erl_drv_mutex_lock(wxe_batch_locker_m); - //fprintf(stderr, "\r\nCB Start ");fflush(stderr); + // fprintf(stderr, "\r\nCB EV Start ");fflush(stderr); app->dispatch_cb(wxe_batch, wxe_batch_cb_saved, process); - //fprintf(stderr, ".. done \r\n");fflush(stderr); + // fprintf(stderr, ".. done \r\n");fflush(stderr); wxe_batch_caller = 0; erl_drv_mutex_unlock(wxe_batch_locker_m); driver_demonitor_process(port, &monitor); } -void handle_callback_batch(ErlDrvPort port) -{ - WxeApp * app = (WxeApp *) wxTheApp; - // Should we be able to handle commands when recursing? probably - erl_drv_mutex_lock(wxe_batch_locker_m); - app->dispatch(wxe_batch, 0, WXE_CALLBACK); - wxe_batch_caller = 0; - erl_drv_mutex_unlock(wxe_batch_locker_m); -} - // Called by wx thread void WxeApp::idle(wxIdleEvent& event) { dispatch_cmds(); @@ -394,8 +385,10 @@ int WxeApp::dispatch(wxList * batch, int blevel, int list_type) case WXE_CB_RETURN: // erl_drv_mutex_unlock(wxe_batch_locker_m); should be called after // whatever cleaning is necessary - memcpy(cb_buff, event->buffer, event->len); - cb_len = event->len; + if(event->len > 0) { + cb_buff = (char *) driver_alloc(event->len); + memcpy(cb_buff, event->buffer, event->len); + } return blevel; default: erl_drv_mutex_unlock(wxe_batch_locker_m); @@ -448,8 +441,10 @@ void WxeApp::dispatch_cb(wxList * batch, wxList * temp, ErlDrvTermData process) case WXE_DEBUG_PING: break; case WXE_CB_RETURN: - memcpy(cb_buff, event->buffer, event->len); - cb_len = event->len; + if(event->len > 0) { + cb_buff = (char *) driver_alloc(event->len); + memcpy(cb_buff, event->buffer, event->len); + } callback_returned = 1; return; case WXE_CB_START: @@ -471,7 +466,7 @@ void WxeApp::dispatch_cb(wxList * batch, wxList * temp, ErlDrvTermData process) } delete event; } else { - // fprintf(stderr, " sav %d \r\n", event->op); + // fprintf(stderr, " save %d \r\n", event->op); temp->Append(event); } } @@ -903,11 +898,13 @@ int wxCALLBACK wxEListCtrlCompare(long item1, long item2, long callbackInfoPtr) rt.addAtom("_wx_invoke_cb_"); rt.addTupleCount(3); rt.send(); - handle_callback_batch(cb->port); + handle_event_callback(cb->port, memenv->owner); - if(((WxeApp *) wxTheApp)->cb_len > 0) { - char * bp = ((WxeApp *) wxTheApp)->cb_buff; - return *(int*) bp; - } else - return 0; + if(((WxeApp *) wxTheApp)->cb_buff) { + int res = * (int*) ((WxeApp *) wxTheApp)->cb_buff; + driver_free(((WxeApp *) wxTheApp)->cb_buff); + ((WxeApp *) wxTheApp)->cb_buff = NULL; + return res; + } + return 0; } diff --git a/lib/wx/c_src/wxe_impl.h b/lib/wx/c_src/wxe_impl.h index dd872745be..ee31068d5d 100644 --- a/lib/wx/c_src/wxe_impl.h +++ b/lib/wx/c_src/wxe_impl.h @@ -178,7 +178,7 @@ public: wxeMemEnv * global_me; // Temp container for callbacks - char cb_buff[256]; + char *cb_buff; int cb_len; }; @@ -195,7 +195,6 @@ class wxETreeItemData : public wxTreeItemData bool sendevent(wxEvent * event, ErlDrvPort port); void pre_callback(); -void handle_callback_batch(ErlDrvPort port); // For wxePrintout void handle_event_callback(ErlDrvPort port, ErlDrvTermData process); void activateGL(ErlDrvTermData caller); diff --git a/lib/wx/src/wxe_server.erl b/lib/wx/src/wxe_server.erl index 40412987a5..69e2189fac 100644 --- a/lib/wx/src/wxe_server.erl +++ b/lib/wx/src/wxe_server.erl @@ -1,19 +1,19 @@ %% %% %CopyrightBegin% -%% -%% Copyright Ericsson AB 2008-2009. All Rights Reserved. -%% +%% +%% Copyright Ericsson AB 2008-2011. All Rights Reserved. +%% %% The contents of this file are subject to the Erlang Public License, %% Version 1.1, (the "License"); you may not use this file except in %% compliance with the License. You should have received a copy of the %% Erlang Public License along with this software. If not, it can be %% retrieved online at http://www.erlang.org/. -%% +%% %% Software distributed under the License is distributed on an "AS IS" %% basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See %% the License for the specific language governing rights and limitations %% under the License. -%% +%% %% %CopyrightEnd% %%%------------------------------------------------------------------- %%% File : wxe_server.erl @@ -24,7 +24,7 @@ %%% Created : 17 Jan 2007 by Dan Gudmundsson %%%------------------------------------------------------------------- -%% @hidden +%% @hidden -module(wxe_server). -behaviour(gen_server). @@ -65,7 +65,7 @@ start() -> end; Env = #wx_env{sv=Pid} -> case erlang:is_process_alive(Pid) of - true -> + true -> Env; false -> %% Ok we got an old wx env, someone forgot erase(?WXE_IDENTIFIER), %% to call wx:destroy() @@ -94,7 +94,7 @@ init([]) -> {ok,#state{port=Port, cb_port=CBPort, users=gb_trees:empty(), cb=gb_trees:empty(), cb_cnt=1}}. -%% Register process +%% Register process handle_call(register_me, {From,_}, State=#state{users=Users}) -> erlang:monitor(process, From), case gb_trees:is_defined(From, Users) of @@ -147,7 +147,7 @@ handle_cast({debug, Level}, State) -> put(?WXE_IDENTIFIER, Env#wx_env{debug=Level}), {noreply, State}; -handle_cast(_Msg, State) -> +handle_cast(_Msg, State) -> ?log("Unknown message ~p sent to ~p~n",[_Msg, ?MODULE]), {noreply, State}. @@ -156,7 +156,7 @@ handle_cast(_Msg, State) -> %% Callback request from driver handle_info(Cb = {_, _, '_wx_invoke_cb_'}, State) -> invoke_cb(Cb, State), - {noreply, State}; + {noreply, State}; handle_info({wx_delete_cb, FunId}, State0 = #state{cb=CB}) when is_integer(FunId) -> case get(FunId) of undefined -> @@ -166,7 +166,7 @@ handle_info({wx_delete_cb, FunId}, State0 = #state{cb=CB}) when is_integer(FunId {noreply, State0#state{cb=gb_trees:delete(Fun, CB)}} end; handle_info({'DOWN',_,process,Pid,_}, State=#state{users=Users0,cleaners=Cs}) -> - try + try User = gb_trees:get(Pid,Users0), Users = gb_trees:delete(Pid,Users0), Env = wx:get_env(), @@ -210,7 +210,7 @@ handle_connect(Object, EvData, From, State0 = #state{users=Users}) -> case Handler0 of #wx_ref{} when Callback =:= 0 -> CBHandler = Handler0, - Handler = Handler0; + Handler = Handler0; undefined when Callback =:= 0 -> Handler = new_evt_listener(State0), CBHandler = Handler; @@ -225,7 +225,7 @@ handle_connect(Object, EvData, From, State0 = #state{users=Users}) -> {FunId, State} = attach_fun(Callback,State1), Res = wxEvtHandler:connect_impl(CBHandler,Object, wxEvtHandler:replace_fun_with_id(EvData,FunId)), - case Res of + case Res of ok -> {reply,Res,State}; _Error -> {reply,Res,State0} end; @@ -238,11 +238,7 @@ invoke_cb({{Ev=#wx{}, Ref=#wx_ref{}}, FunId,_}, _S) -> %% Event callbacks case get(FunId) of Fun when is_function(Fun) -> - invoke_callback(fun() -> - wxe_util:cast(?WXE_CB_START, <<>>), - Fun(Ev, Ref), - <<>> - end); + invoke_callback(fun() -> Fun(Ev, Ref), <<>> end); Err -> ?log("Internal Error ~p~n",[Err]) end; @@ -254,12 +250,14 @@ invoke_cb({FunId, Args, _}, _S) when is_list(Args), is_integer(FunId) -> Err -> ?log("Internal Error ~p ~p ~p~n",[Err, FunId, Args]) end. - + invoke_callback(Fun) -> Env = get(?WXE_IDENTIFIER), CB = fun() -> wx:set_env(Env), - Res = try Return = Fun(), + wxe_util:cast(?WXE_CB_START, <<>>), + Res = try + Return = Fun(), true = is_binary(Return), Return catch _:Reason -> @@ -278,9 +276,9 @@ new_evt_listener(State) -> get_result(State). get_result(_State) -> - receive + receive {'_wxe_result_', Res} -> Res; - {'_wxe_error_', Op, Error} -> + {'_wxe_error_', Op, Error} -> erlang:error({Error, {wxEvtHandler, {internal_installer, Op}}}) end. @@ -289,7 +287,7 @@ attach_fun(Fun, S = #state{cb=CB,cb_cnt=Next}) -> {value, ID} -> {ID,S}; none -> - put(Next,Fun), + put(Next,Fun), {Next,S#state{cb=gb_trees:insert(Fun,Next,CB),cb_cnt=Next+1}} end. @@ -297,7 +295,7 @@ handle_disconnect(Object, Evh, From, State0 = #state{users=Users0}) -> User0 = #user{events=Evs0, evt_handler=PidH} = gb_trees:get(From, Users0), Fun = wxEvtHandler:get_callback(Evh), case find_handler(Evs0, Object, Fun) of - [] -> + [] -> {reply, false, State0}; Handlers -> case disconnect(Object,Evh, Handlers) of @@ -310,7 +308,7 @@ handle_disconnect(Object, Evh, From, State0 = #state{users=Users0}) -> [] when PidH =/= undefined -> wxEvtHandler:destroy_evt_listener(PidH), User0#user{events=[], evt_handler=undefined}; - Evs -> + Evs -> User0#user{events=Evs} end, {reply, true, State0#state{users=gb_trees:update(From,User,Users0)}}; @@ -345,7 +343,7 @@ find_handler([],_Object,_Fun,Res) -> %% Cleanup -%% The server handles callbacks from driver so every other wx call must +%% The server handles callbacks from driver so every other wx call must %% be called from another process, therefore the cleaning must be spawned. %% cleanup(Env, _Pid, Data) -> @@ -358,7 +356,7 @@ 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), case is_function(CB) of - true -> + true -> wxEvtHandler:destroy_evt_listener(CbH); false -> ignore diff --git a/lib/wx/test/wx_class_SUITE.erl b/lib/wx/test/wx_class_SUITE.erl index ef37dc68bc..00ef1289ab 100644 --- a/lib/wx/test/wx_class_SUITE.erl +++ b/lib/wx/test/wx_class_SUITE.erl @@ -390,7 +390,7 @@ listCtrlVirtual(Config) -> end}, {onGetItemAttr, fun(_This, Item) when Item rem 3 == 0 -> IA; - (_This, Item) -> + (_This, _Item) -> wx:typeCast(wx:null(), wxListItemAttr) end}, {onGetItemColumnImage, fun(_This, Item, 1) -> -- cgit v1.2.3