"SocketReactor::addEventHandler" and "SocketReactor::removeEventHandler" must protect the access to "NotifierPtr pNotifier"

[sebastien-guay]

"SocketReactor::addEventHandler" and "SocketReactor::removeEventHandler" must protect the access to "NotifierPtr pNotifier"

Those two methods already use FastMutex::ScopedLock lock(_mutex), but the scope is not large enough to protect "NotifierPtr pNotifier" that is accessed by calling SocketNotifier::addObserver() and SocketNotifier::removeObserver().

It is mentioned in SocketReator.h that it is safe to call addEventHandler() and removeEventHandler() from another thread while the SocketReactor is running. My current use of the SocketReactor encountered an issue where the SocketNotifier::_events has been corrupted by a concurent write access done by SocketReactor::addEventHandler() and SocketReactor::removeEventHandler().
The call stack show that the SocketReactor::addEventHandler is stuck in a while loop in gcc/libstdc++/tree.cc Rb_tree_insert_and_rebalance()

I clearly see in my logs that it happened while my SocketConnector::unregisterConnector() and SocketConnector::registerConnector() were called by two different threads.

#0 0x00a80a7b in std::_Rb_tree_insert_and_rebalance () from /usr/lib/libstdc++.so.6
#1 0x06ccb430 in std::_Rb_tree<Poco::Net::SocketNotification*, Poco::Net::SocketNotification*, std::_IdentityPoco::Net::SocketNotification*, std::lessPoco::Net::SocketNotification*, std::allocatorPoco::Net::SocketNotification* >::_M_insert (this=0xac75dc90, __x=0x0, __p=0xac7ed0c8, __v=@0xb5fb0c40) at /usr/lib/gcc/i386-redhat-linux/3.4.6/../../../../include/c++/3.4.6/bits/stl_tree.h:816
#2 0x06ccb15d in std::_Rb_tree<Poco::Net::SocketNotification*, Poco::Net::SocketNotification*, std::_IdentityPoco::Net::SocketNotification*, std::lessPoco::Net::SocketNotification*, std::allocatorPoco::Net::SocketNotification* >::insert_equal (this=0xac75dc90, __v=@0xb5fb0c40) at /usr/lib/gcc/i386-redhat-linux/3.4.6/../../../../include/c++/3.4.6/bits/stl_tree.h:858
#3 0x06ccad86 in std::multiset<Poco::Net::SocketNotification*, std::lessPoco::Net::SocketNotification*, std::allocatorPoco::Net::SocketNotification* >::insert (this=0xac75dc90, __x=@0xb5fb0c40)
at /usr/lib/gcc/i386-redhat-linux/3.4.6/../../../../include/c++/3.4.6/bits/stl_multiset.h:306
#4 0x06cca72b in Poco::Net::SocketNotifier::addObserver (this=0xac75dc70, pReactor=0x8d27958, observer=@0xb5fb0cd0) at src/SocketNotifier.cpp:45
#5 0x06cd060f in Poco::Net::SocketReactor::addEventHandler (this=0x8d27958, socket=@0xac7f5a74, observer=@0xb5fb0cd0) at src/SocketReactor.cpp:178

SocketReactor::run() is already protecting the access to the SocketNotifier.