SocketNotifier not thread-safe pocoproject#2345

Romain-Geissler-1A · May 31, 2018 · 45903ed · 45903ed
1 parent b539f55
commit 45903ed
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 13 deletions.
diff --git a/Net/include/Poco/Net/SocketNotifier.h b/Net/include/Poco/Net/SocketNotifier.h
@@ -70,10 +70,13 @@ class Net_API SocketNotifier: public Poco::RefCountedObject
 
 private:
 	typedef std::multiset<SocketNotification*> EventSet;
+	typedef Poco::FastMutex                    MutexType;
+	typedef MutexType::ScopedLock              ScopedLock;
 
 	EventSet                 _events;
 	Poco::NotificationCenter _nc;
 	Socket                   _socket;
+	MutexType                _mutex;
 };
 
 
@@ -82,6 +85,7 @@ class Net_API SocketNotifier: public Poco::RefCountedObject
 //
 inline bool SocketNotifier::accepts(SocketNotification* pNotification)
 {
+	ScopedLock l(_mutex);
 	return _events.find(pNotification) != _events.end();
 }
 

diff --git a/Net/src/PollSet.cpp b/Net/src/PollSet.cpp
@@ -162,7 +162,10 @@ class PollSetImpl
 	{
 		PollSet::SocketModeMap result;
 
-		if(_socketMap.empty()) return result;
+		{
+			Poco::FastMutex::ScopedLock lock(_mutex);
+			if(_socketMap.empty()) return result;
+		}
 
 		Poco::Timespan remainingTime(timeout);
 		int rc;

diff --git a/Net/src/SocketNotifier.cpp b/Net/src/SocketNotifier.cpp
@@ -35,6 +35,7 @@ SocketNotifier::~SocketNotifier()
 void SocketNotifier::addObserver(SocketReactor* pReactor, const Poco::AbstractObserver& observer)
 {
 	_nc.addObserver(observer);
+	ScopedLock l(_mutex);
 	if (observer.accepts(pReactor->_pReadableNotification))
 		_events.insert(pReactor->_pReadableNotification.get());
 	else if (observer.accepts(pReactor->_pWritableNotification))
@@ -49,6 +50,7 @@ void SocketNotifier::addObserver(SocketReactor* pReactor, const Poco::AbstractOb
 void SocketNotifier::removeObserver(SocketReactor* pReactor, const Poco::AbstractObserver& observer)
 {
 	_nc.removeObserver(observer);
+	ScopedLock l(_mutex);
 	EventSet::iterator it = _events.end();
 	if (observer.accepts(pReactor->_pReadableNotification))
 		it = _events.find(pReactor->_pReadableNotification.get());

diff --git a/Net/testsuite/src/SocketReactorTest.cpp b/Net/testsuite/src/SocketReactorTest.cpp
@@ -52,13 +52,10 @@ namespace
 			_reactor(reactor)
 		{
 			_reactor.addEventHandler(_socket, Observer<EchoServiceHandler, ReadableNotification>(*this, &EchoServiceHandler::onReadable));
-			_reactor.addEventHandler(_socket, Observer<EchoServiceHandler, ShutdownNotification>(*this, &EchoServiceHandler::onShutdown));
 		}
 
 		~EchoServiceHandler()
 		{
-			_reactor.removeEventHandler(_socket, Observer<EchoServiceHandler, ReadableNotification>(*this, &EchoServiceHandler::onReadable));
-			_reactor.removeEventHandler(_socket, Observer<EchoServiceHandler, ShutdownNotification>(*this, &EchoServiceHandler::onShutdown));
 		}
 
 		void onReadable(ReadableNotification* pNf)
@@ -70,13 +67,11 @@ namespace
 			{
 				_socket.sendBytes(buffer, n);
 			}
-			else delete this;
-		}
-
-		void onShutdown(ShutdownNotification* pNf)
-		{
-			pNf->release();
-			delete this;
+			else
+			{
+				_reactor.removeEventHandler(_socket, Observer<EchoServiceHandler, ReadableNotification>(*this, &EchoServiceHandler::onReadable));
+				delete this;
+			}
 		}
 
 	private:
@@ -129,8 +124,11 @@ namespace
 				checkReadableObserverCount(1);
 				_reactor.removeEventHandler(_socket, Observer<ClientServiceHandler, ReadableNotification>(*this, &ClientServiceHandler::onReadable));
 				checkReadableObserverCount(0);
-				if (_once || _data.size() == 8192) _reactor.stop();
-				delete this;
+				if (_once || _data.size() == 8192)
+				{
+					_reactor.stop();
+					delete this;
+				}
 			}
 		}