Bug #2313
closedUndefined behavior in UtilSignal::DestructInHandler test case
100%
Description
Reduced testcase:
unique_ptr<SignalOwner0> so(new SignalOwner0);
so->sig.connect([&] {
so.reset();
});
BOOST_CHECK_NO_THROW(so->emitSignal(sig));
The .reset()
in the handler causes the deallocation of the signal, therefore everything that happens in Signal::operator()
after this handler has run is undefined behavior.
Deallocating a signal from one of its handler should be illegal since there's always at least one statement that uses the this
pointer (m_isExecuting = false;
) after the execution of any handler.
Updated by Alex Afanasyev over 9 years ago
- Related to Bug #2302: UtilSignal/DisconnectSelfInHandler segfault added
Updated by Alex Afanasyev over 9 years ago
I believe this is exactly the problem I was having in #2302
Updated by Davide Pesavento over 9 years ago
I don't see an apparent similarity between the two test cases (in DisconnectSelfInHandler
the SignalOwner0
instance is statically allocated on the stack), can you clarify?
Updated by Junxiao Shi over 9 years ago
- Related to deleted (Bug #2302: UtilSignal/DisconnectSelfInHandler segfault)
Updated by Junxiao Shi over 9 years ago
- Category changed from Tests to Utils
- Estimated time set to 1.00 h
This is unrelated to #2302. #2302 is an isolated problem in a test case, while this Bug is more than a test case problem.
Allow destructing a Signal from its handler comes from the following scenario:
In NFD Face::onFail
, a handler from FaceTable
may want to remove the failing face from the FaceTable.
FaceTable may hold the last shared_ptr
to the face. Removing it causes the destruction of the face and thus the Signal.
We can't drop this feature without finding a solution to this scenario.
Updated by Davide Pesavento over 9 years ago
Maybe the onFail
part of NFD's face system should be redesigned so that it no longer depends on this "feature"..?
Updated by Junxiao Shi over 9 years ago
I found a way to solve note-5 problem: FaceTable::remove
can use io.post
to delay deleting the Face from container, thus Signal isn't destructed during its execution.
Updated by Junxiao Shi about 9 years ago
Confirmed with valgrind.
==2596== at 0x6C2891: ndn::util::signal::Signal<ndn::util::signal::tests::UtilSignal::SignalOwner0, >::operator()() (signal-signal.hpp:239)
==2596== by 0x6C0CCB: ndn::util::signal::Signal<ndn::util::signal::tests::UtilSignal::SignalOwner0, >::operator()(ndn::util::signal::DummyExtraArg const&) (signal-signal.hpp:246)
==2596== by 0x6BE6D8: void ndn::util::signal::tests::UtilSignal::SignalOwner0::emit_sig<ndn::util::signal::DummyExtraArg>(ndn::util::signal::DummyExtraArg const&) (signal.cpp:39)
==2596== by 0x6B8C75: ndn::util::signal::tests::UtilSignal::DestructInHandler::test_method() (signal.cpp:435)
==2596== by 0x6B8B43: ndn::util::signal::tests::UtilSignal::DestructInHandler_invoker() (signal.cpp:425)
==2596== by 0x4D6879: boost::unit_test::ut_detail::unused boost::unit_test::ut_detail::invoker<boost::unit_test::ut_detail::unused>::invoke<void (*)()>(void (*&)()) (callback.hpp:56)
==2596== by 0x4D5E63: boost::unit_test::ut_detail::callback0_impl_t<boost::unit_test::ut_detail::unused, void (*)()>::invoke() (callback.hpp:89)
==2596== by 0x5BDF2F0: ??? (in /usr/lib/libboost_unit_test_framework.so.1.48.0)
==2596== by 0x5BC34FD: boost::execution_monitor::catch_signals(boost::unit_test::callback0<int> const&) (in /usr/lib/libboost_unit_test_framework.so.1.48.0)
==2596== by 0x5BC3DFA: boost::execution_monitor::execute(boost::unit_test::callback0<int> const&) (in /usr/lib/libboost_unit_test_framework.so.1.48.0)
==2596== by 0x5BDF3EA: boost::unit_test::unit_test_monitor_t::execute_and_translate(boost::unit_test::test_case const&) (in /usr/lib/libboost_unit_test_framework.so.1.48.0)
==2596== by 0x5BCA8A0: boost::unit_test::framework_impl::visit(boost::unit_test::test_case const&) (in /usr/lib/libboost_unit_test_framework.so.1.48.0)
==2596== Address 0x97e6c60 is 16 bytes inside a block of size 32 free'd
==2596== at 0x4C2A4BC: operator delete(void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==2596== by 0x6C206D: std::default_delete<ndn::util::signal::tests::UtilSignal::SignalOwner0>::operator()(ndn::util::signal::tests::UtilSignal::SignalOwner0*) const (unique_ptr.h:63)
==2596== by 0x6BFE1B: std::unique_ptr<ndn::util::signal::tests::UtilSignal::SignalOwner0, std::default_delete<ndn::util::signal::tests::UtilSignal::SignalOwner0> >::reset(ndn::util::signal::tests::UtilSignal::SignalOwner0*) (unique_ptr.h:245)
==2596== by 0x6B8B74: _ZZN3ndn4util6signal5tests10UtilSignal17DestructInHandler11test_methodEvENKUlvE_clEv (signal.cpp:432)
==2596== by 0x6BB8C9: _ZNSt17_Function_handlerIFvvEZN3ndn4util6signal5tests10UtilSignal17DestructInHandler11test_methodEvEUlvE_E9_M_invokeERKSt9_Any_data (functional:1778)
==2596== by 0x69C8F1: std::function<void ()()>::operator()() const (functional:2161)
==2596== by 0x6C2881: ndn::util::signal::Signal<ndn::util::signal::tests::UtilSignal::SignalOwner0, >::operator()() (signal-signal.hpp:232)
==2596== by 0x6C0CCB: ndn::util::signal::Signal<ndn::util::signal::tests::UtilSignal::SignalOwner0, >::operator()(ndn::util::signal::DummyExtraArg const&) (signal-signal.hpp:246)
==2596== by 0x6BE6D8: void ndn::util::signal::tests::UtilSignal::SignalOwner0::emit_sig<ndn::util::signal::DummyExtraArg>(ndn::util::signal::DummyExtraArg const&) (signal.cpp:39)
==2596== by 0x6B8C75: ndn::util::signal::tests::UtilSignal::DestructInHandler::test_method() (signal.cpp:435)
==2596== by 0x6B8B43: ndn::util::signal::tests::UtilSignal::DestructInHandler_invoker() (signal.cpp:425)
==2596== by 0x4D6879: boost::unit_test::ut_detail::unused boost::unit_test::ut_detail::invoker<boost::unit_test::ut_detail::unused>::invoke<void (*)()>(void (*&)()) (callback.hpp:56)
==2596==
Updated by Davide Pesavento about 9 years ago
- Blocks Task #2589: CI: enable AddressSanitizer for unit tests added
Updated by Junxiao Shi about 9 years ago
- Status changed from New to In Progress
- Target version changed from v0.3 to v0.4
Updated by Junxiao Shi about 9 years ago
- Status changed from In Progress to Code review
- % Done changed from 0 to 100
http://gerrit.named-data.net/1820 forbids destruction of Signal during signal emission.
nfd::FaceTable
doesn't seem affected.
Updated by Junxiao Shi about 9 years ago
- Status changed from Code review to Closed