summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTobias Markmann <tm@ayena.de>2015-02-07 23:48:10 (GMT)
committerSwift Review <review@swift.im>2015-02-09 11:09:23 (GMT)
commit7d3df55bf5fd93b3eaca36cc43cb22ea7879bf7e (patch)
tree6e48bd5233498068609f4c3e62eda25a2acfe4d6 /Swiften/EventLoop
parent2b81680fc59dd3170635967948727b4dc49b951e (diff)
downloadswift-7d3df55bf5fd93b3eaca36cc43cb22ea7879bf7e.zip
swift-7d3df55bf5fd93b3eaca36cc43cb22ea7879bf7e.tar.bz2
Fix data race in DummyEventLoop and BoostConnection(Server)Test reported by TSAN
The data race is on the events_ member in DummyEventLoop. A BoostIOServerThread can post events to the DummyEventLoop and thereby access its events_ data member while the test's main code processes events of the loop. To prevent access to the DummyEventLoop by the BoostIOServiceThread after the DummyEventLoop is deleted, the BoostIOServiceThread is deleted before the DummyEventLoop. Process remaining events in BoostConnectionTest::tearDown like we do in BoostConnectionServerTest::tearDown. Test-Information: Run multiple times on NetworkTest as TSAN enabled build on Linux. Without the patch it reports a data race for the events_ deque member. Change-Id: I3c85535338fc0ce0263dbfc3534aceb1dd09c137
Diffstat (limited to 'Swiften/EventLoop')
-rw-r--r--Swiften/EventLoop/DummyEventLoop.cpp3
-rw-r--r--Swiften/EventLoop/DummyEventLoop.h27
2 files changed, 26 insertions, 4 deletions
diff --git a/Swiften/EventLoop/DummyEventLoop.cpp b/Swiften/EventLoop/DummyEventLoop.cpp
index 4ea425e..b8e631e 100644
--- a/Swiften/EventLoop/DummyEventLoop.cpp
+++ b/Swiften/EventLoop/DummyEventLoop.cpp
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2010 Isode Limited.
+ * Copyright (c) 2010-2015 Isode Limited.
* All rights reserved.
* See the COPYING file for more information.
*/
@@ -14,6 +14,7 @@ DummyEventLoop::DummyEventLoop() {
}
DummyEventLoop::~DummyEventLoop() {
+ boost::lock_guard<boost::mutex> lock(eventsMutex_);
if (!events_.empty()) {
std::cerr << "DummyEventLoop: Unhandled events at destruction time" << std::endl;
}
diff --git a/Swiften/EventLoop/DummyEventLoop.h b/Swiften/EventLoop/DummyEventLoop.h
index 6ee1b77..297549e 100644
--- a/Swiften/EventLoop/DummyEventLoop.h
+++ b/Swiften/EventLoop/DummyEventLoop.h
@@ -8,6 +8,9 @@
#include <deque>
+#include <boost/thread/mutex.hpp>
+#include <boost/thread/locks.hpp>
+
#include <Swiften/Base/API.h>
#include <Swiften/EventLoop/EventLoop.h>
@@ -18,21 +21,39 @@ namespace Swift {
virtual ~DummyEventLoop();
void processEvents() {
- while (!events_.empty()) {
- handleEvent(events_[0]);
- events_.pop_front();
+ while (hasEvents()) {
+ /*
+ Creating a copy of the to-be-handled Event object because handling
+ it can result in a DummyEventLoop::post() call.
+ This call would also try to lock the eventsMutex_, resulting in a
+ deadlock.
+ */
+
+ eventsMutex_.lock();
+ Event eventCopy = events_[0];
+ eventsMutex_.unlock();
+
+ handleEvent(eventCopy);
+
+ {
+ boost::lock_guard<boost::mutex> lock(eventsMutex_);
+ events_.pop_front();
+ }
}
}
bool hasEvents() {
+ boost::lock_guard<boost::mutex> lock(eventsMutex_);
return !events_.empty();
}
virtual void post(const Event& event) {
+ boost::lock_guard<boost::mutex> lock(eventsMutex_);
events_.push_back(event);
}
private:
+ boost::mutex eventsMutex_;
std::deque<Event> events_;
};
}