From f44805dc16feb213f65a65c9a74f1e3125976c2f Mon Sep 17 00:00:00 2001 From: Dan Gudmundsson Date: Wed, 16 Nov 2011 15:03:31 +0100 Subject: [wx] Fix deadlock in callback handling New testcase showcase the deadlock --- lib/wx/c_src/wxe_impl.cpp | 24 ++++++++++++++++++++---- lib/wx/test/wx_event_SUITE.erl | 34 +++++++++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 5 deletions(-) (limited to 'lib') diff --git a/lib/wx/c_src/wxe_impl.cpp b/lib/wx/c_src/wxe_impl.cpp index e430fbc7a2..69fcd4e362 100644 --- a/lib/wx/c_src/wxe_impl.cpp +++ b/lib/wx/c_src/wxe_impl.cpp @@ -331,9 +331,9 @@ 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 EV Start ");fflush(stderr); + //fprintf(stderr, "\r\nCB EV Start %lu \r\n", process);fflush(stderr); app->dispatch_cb(wxe_batch, wxe_batch_cb_saved, process); - // fprintf(stderr, ".. done \r\n");fflush(stderr); + //fprintf(stderr, "CB EV done %lu \r\n", process);fflush(stderr); wxe_batch_caller = 0; erl_drv_mutex_unlock(wxe_batch_locker_m); driver_demonitor_process(port, &monitor); @@ -430,8 +430,9 @@ void WxeApp::dispatch_cb(wxList * batch, wxList * temp, ErlDrvTermData process) wxeCommand *event = (wxeCommand *)node->GetData(); wxeMemEnv *memenv = getMemEnv(event->port); batch->Erase(node); + // fprintf(stderr, " Ev %d %lu\r\n", event->op, event->caller); if(event->caller == process || // Callbacks from CB process only - event->op == WXE_CB_START || // Recursive event callback allow + event->op == WXE_CB_START || // Event callback start change process // Allow connect_cb during CB i.e. msg from wxe_server. (memenv && event->caller == memenv->owner)) { @@ -453,6 +454,7 @@ void WxeApp::dispatch_cb(wxList * batch, wxList * temp, ErlDrvTermData process) break; default: erl_drv_mutex_unlock(wxe_batch_locker_m); + size_t start=temp->GetCount(); if(event->op < OPENGL_START) { // fprintf(stderr, " cb %d \r\n", event->op); wxe_dispatch(*event); @@ -460,9 +462,23 @@ void WxeApp::dispatch_cb(wxList * batch, wxList * temp, ErlDrvTermData process) gl_dispatch(event->op,event->buffer,event->caller,event->bin); } erl_drv_mutex_lock(wxe_batch_locker_m); - break; + if(temp->GetCount() > start) { + // We have recursed dispatch_cb and messages for this + // callback may be saved on temp list move them + // to orig list + for(wxList::compatibility_iterator node = temp->Item(start); + node; + node = node->GetNext()) { + wxeCommand *ev = (wxeCommand *)node->GetData(); + if(ev->caller == process) { + batch->Append(ev); + temp->Erase(node); + } + } + } if(callback_returned) return; + break; } delete event; } else { diff --git a/lib/wx/test/wx_event_SUITE.erl b/lib/wx/test/wx_event_SUITE.erl index 0d8dd4852e..8f364049b4 100644 --- a/lib/wx/test/wx_event_SUITE.erl +++ b/lib/wx/test/wx_event_SUITE.erl @@ -47,7 +47,7 @@ suite() -> [{ct_hooks,[ts_install_cth]}]. all() -> [connect, disconnect, connect_msg_20, connect_cb_20, - mouse_on_grid, spin_event, connect_in_callback]. + mouse_on_grid, spin_event, connect_in_callback, recursive]. groups() -> []. @@ -331,3 +331,35 @@ connect_in_callback(Config) -> wx_test_lib:flush(), wx_test_lib:wx_destroy(Frame, Config). + +%% Test that event callback which triggers another callback works +%% i.e. the callback invoker in driver will recurse +recursive(TestInfo) when is_atom(TestInfo) -> wx_test_lib:tc_info(TestInfo); +recursive(Config) -> + Wx = wx:new(), + Frame = wxFrame:new(Wx, ?wxID_ANY, "Connect in callback"), + Panel = wxPanel:new(Frame, []), + Sz = wxBoxSizer:new(?wxVERTICAL), + ListBox = wxListBox:new(Panel, ?wxID_ANY, [{choices, ["foo", "bar", "baz"]}]), + wxSizer:add(Sz, ListBox, [{proportion, 1},{flag, ?wxEXPAND}]), + wxWindow:setSizer(Panel, Sz), + wxListBox:connect(ListBox, command_listbox_selected, + [{callback, + fun(#wx{event=#wxCommand{commandInt=Id}}, _) -> + io:format("Selected ~p~n",[Id]) + end}]), + wxListBox:setSelection(ListBox, 0), + wxListBox:connect(ListBox, size, + [{callback, + fun(#wx{event=#wxSize{}}, _) -> + io:format("Size init ~n",[]), + case wxListBox:getCount(ListBox) > 0 of + true -> wxListBox:delete(ListBox, 0); + false -> ok + end, + io:format("Size done ~n",[]) + end}]), + wxFrame:show(Frame), + wx_test_lib:flush(), + + wx_test_lib:wx_destroy(Frame, Config). -- cgit v1.2.3