From 5043dcb830e96aa11b04e3bebc935c1c4cc606f2 Mon Sep 17 00:00:00 2001 From: Vitor Sousa Date: Mon, 26 Jan 2015 15:49:16 -0200 Subject: [PATCH] eo_cxx: Fix signal_connection disconnect crash Fixed crash when disconnecting event inside of its own event callback. Instead of deleting the callback object immediately during disconnection (which causes the callback to be freed), the deletion is now scheduled for later (using ecore_main_loop_thread_safe_call_async). Updated some Makefiles to proper include ecore now that it is used in all event wrappers. Added a unit test to verify crashes under these circumstances. --- src/Makefile_Eina_Cxx.am | 1 + src/Makefile_Eolian_Cxx.am | 2 +- src/bindings/eo_cxx/eo_event.hh | 9 +++- .../eolian_cxx/eolian_cxx_test_callback.cc | 54 +++++++++++++++++++ 4 files changed, 64 insertions(+), 2 deletions(-) diff --git a/src/Makefile_Eina_Cxx.am b/src/Makefile_Eina_Cxx.am index 4607ecef1c..70ef18a7bd 100644 --- a/src/Makefile_Eina_Cxx.am +++ b/src/Makefile_Eina_Cxx.am @@ -64,6 +64,7 @@ tests_eina_cxx_eina_cxx_suite_CXXFLAGS = -I$(top_builddir)/src/lib/efl \ -DTESTS_BUILD_DIR=\"$(top_builddir)/src/tests/eina_cxx\" \ @CHECK_CFLAGS@ \ @EO_CFLAGS@ \ +@ECORE_CFLAGS@ \ @EO_CXX_CFLAGS@ \ @EINA_CXX_CFLAGS@ tests_eina_cxx_eina_cxx_suite_LDADD = @CHECK_LIBS@ @USE_EINA_LIBS@ @USE_EO_LIBS@ diff --git a/src/Makefile_Eolian_Cxx.am b/src/Makefile_Eolian_Cxx.am index 6dff2671df..d086bff103 100644 --- a/src/Makefile_Eolian_Cxx.am +++ b/src/Makefile_Eolian_Cxx.am @@ -118,7 +118,7 @@ tests_eolian_cxx_eolian_cxx_suite_CXXFLAGS = \ tests_eolian_cxx_eolian_cxx_suite_CFLAGS = ${tests_eolian_cxx_eolian_cxx_suite_CXXFLAGS} tests_eolian_cxx_eolian_cxx_suite_CPPFLAGS = ${tests_eolian_cxx_eolian_cxx_suite_CXXFLAGS} tests_eolian_cxx_eolian_cxx_suite_LDADD = \ -@CHECK_LIBS@ @USE_EO_LIBS@ @USE_EINA_LIBS@ @USE_EOLIAN_LIBS@ +@CHECK_LIBS@ @USE_EO_LIBS@ @USE_EINA_LIBS@ @USE_ECORE_LIBS@ @USE_EOLIAN_LIBS@ tests_eolian_cxx_eolian_cxx_suite_DEPENDENCIES = @USE_EOLIAN_INTERNAL_LIBS@ endif diff --git a/src/bindings/eo_cxx/eo_event.hh b/src/bindings/eo_cxx/eo_event.hh index f3d2e516d7..44c66afcb6 100644 --- a/src/bindings/eo_cxx/eo_event.hh +++ b/src/bindings/eo_cxx/eo_event.hh @@ -7,6 +7,7 @@ #define EFL_CXX_EO_EVENT_HH #include +#include #include #include @@ -95,7 +96,13 @@ struct _event_deleter void operator()() const { eo_do(_eo, ::eo_event_callback_del(_description, _cb, _data)); - delete _data; + ::ecore_main_loop_thread_safe_call_async(&_deleter_call, _data); + } + +private: + static void _deleter_call(void* data) + { + delete static_cast(data); } F* _data; diff --git a/src/tests/eolian_cxx/eolian_cxx_test_callback.cc b/src/tests/eolian_cxx/eolian_cxx_test_callback.cc index 2acfd09e63..42caa4a34c 100644 --- a/src/tests/eolian_cxx/eolian_cxx_test_callback.cc +++ b/src/tests/eolian_cxx/eolian_cxx_test_callback.cc @@ -10,6 +10,11 @@ #include +#include +#include +#include +#include + void foo(void*) {} START_TEST(eolian_cxx_test_callback_method) @@ -121,6 +126,54 @@ START_TEST(eolian_cxx_test_global_callback) } END_TEST +START_TEST(eolian_cxx_test_disconnect_inside_callback) +{ + efl::eo::eo_init i; + callback c; + + std::vector capture_me; + int times_called = 0; + + ::efl::eo::signal_connection sig(nullptr); + sig = c.callback_callback_add_add( + std::bind([&sig, &capture_me, ×_called](void *info) + { + ++times_called; + std::cout << "times_called: " << times_called << std::endl; + std::cout << "&sig: " << &sig << std::endl; + if (times_called <= 1) + return; + + sig.disconnect(); + + long* info_l = static_cast(info); + std::cout << "info: " << info << std::endl; + std::cout << "*info_l: " << *info_l << std::endl; + + fail_if(*info_l != 42); + + capture_me = {9, 0, 8, 1, 7, 2, 6, 3, 5, 4}; + std::sort(capture_me.begin(), capture_me.end()); + + capture_me[0] = capture_me[1] + +capture_me[2] + capture_me[9]; + + std::cout << "&capture_me: " << &capture_me << std::endl; + std::cout << "capture_me [0] [9]: [" << capture_me[0] << "] ["<< capture_me[9] << "]" << std::endl; + + fail_if(capture_me.size() != 10); + fail_if(capture_me[0] != 12); + fail_if(times_called != 2); + }, std::placeholders::_3)); + + long n = 42; + c.callback_callback_add_call(&n); + + fail_if(capture_me.size() != 10); + fail_if(capture_me[0] != 12); + fail_if(times_called != 2); +} +END_TEST + void eolian_cxx_test_callback(TCase* tc) { @@ -128,4 +181,5 @@ eolian_cxx_test_callback(TCase* tc) tcase_add_test(tc, eolian_cxx_test_callback_event_add); tcase_add_test(tc, eolian_cxx_test_callback_event_del); tcase_add_test(tc, eolian_cxx_test_global_callback); + tcase_add_test(tc, eolian_cxx_test_disconnect_inside_callback); }