Net: Deadlock in SocketReactor when removing handler from callback #4970
Description
Describe the bug
Dead lock when invoking removeEventHandler() from another thread and at the same time invoke removeEventHandler() from event handlers(the SocketReactor thread).
A comment in SocketReactor.h:
/// It is safe to call addEventHandler() and removeEventHandler() from another
/// thread while the SocketReactor is running. Also, it is safe to call
/// addEventHandler() and removeEventHandler() from event handlers.
It seems is not safe .
To Reproduce
thread1:
{
m_reactor.removeEventHandler(
socket1, Poco::NObserver<TcpClient, Poco::Net::ReadableNotification>(*this, &TcpClient::OnReadable));
}
thread2:
Thread2 is a thread which the reactor runs on, and if the socket1 is readable, the reactor will notify the observer by invoking the TcpClient::OnReadable:
void TcpClient::OnReadable(const Poco::AutoPtr<Poco::Net::ReadableNotification> &pNf)
{
m_reactor.removeEventHandler(pNf->socket(),
Poco::NObserver<TcpClient, Poco::Net::ReadableNotification>(*this, &TcpClient::OnReadable));
}
besides the "pNf->socket()" is same to socket1 in thread1.
Reason analyze personally:
// Notification.cpp
void NotificationCenter::removeObserver(const AbstractObserver& observer)
{
Mutex::ScopedLock lock(_mutex);
for (ObserverList::iterator it = _observers.begin(); it != _observers.end(); ++it)
{
if (observer.equals(**it))
{
(*it)->disable();
_observers.erase(it);
return;
}
}
}
// NObserver.h
void notify(Notification* pNf) const
{
Poco::Mutex::ScopedLock lock(_mutex);
if (_pObject)
{
N* pCastNf = dynamic_cast<N*>(pNf);
if (pCastNf)
{
NotificationPtr ptr(pCastNf, true);
(_pObject->*_method)(ptr);
}
}
}
void disable()
{
Poco::Mutex::ScopedLock lock(_mutex);
_pObject = 0;
}
thread 1 has owned NotificationCenter::_mutex when invoking NotificationCenter::removeObserver,then invokeing NObserver::disable() need to own NObserver::_mutex, but this lock is owned by thread2 whick has invoked NObserver::notify();So thread1 is blocked;
In thread2, invoking removeEventHandler() also need to own NotificationCenter::_mutex which has be owned by thread1, so thread2 is blocked;
Expected behavior
thread1 and thread2 both run normally.
Please add relevant environment information:
- OS Type and Version:Windows11
- POCO Version:1.13.3-release
- Third-party product (eg. database or library) type and version
Additional context
Add any other context about the problem here.