libtransport.git Changeset - f328e80a974a

Changeset - f328e80a974a

Parent rev.

Child rev.

[Not reviewed]

0 2 0

Vladimír Matěna - 9 years ago 2016-09-16 23:45:17
vlada.matena@gmail.com

Fix possible double free in ThreadPool

This makes HTTPRequestTest::GETThreadPool test pass deterministicaly.

Problem was static slot used to cleanup threads. When ThreadPool was instantiated
multiple times, threads were also clened up multiple times resulting in segfault.

2 files changed with 11 insertions and 14 deletions:

include/transport/ThreadPool.h

libtransport/ThreadPool.cpp

0 comments (0 inline, 0 general)

include/transport/ThreadPool.h

➞

Show inline comments

@@ @@ -56,8 +56,8 @@ class ThreadPool @@
 	boost::mutex criticalregion;
 	Swift::EventLoop *loop;
 	boost::signals2::signal  < void () > onWorkerAvailable;
 	boost::signals2::signal < void () > onWorkerAvailable;
 	public:
 	ThreadPool(Swift::EventLoop *loop, int maxthreads);
 	~ThreadPool();
@@ @@ -68,6 +68,7 @@ class ThreadPool @@
 	void scheduleFromQueue();
 	int getFreeThread();
 	void releaseThread(int i);
 	void workerBody(Thread *t, int wid);
 };
+}

libtransport/ThreadPool.cpp

➞

Show inline comments

@@ @@ -6,15 +6,6 @@ @@
 namespace Transport {
 DEFINE_LOGGER(logger, "ThreadPool")
 boost::signals2::signal< void (Thread*, int) > onWorkCompleted;
 static void Worker(Thread *t, int wid, Swift::EventLoop *loop)
+{
 	LOG4CXX_INFO(logger, "Starting thread " << wid)
 	t->run();
 	loop->postEvent(boost::bind(boost::ref(onWorkCompleted), t, wid), SWIFTEN_SHRPTR_NAMESPACE::shared_ptr<Swift::EventOwner>());
+}
 ThreadPool::ThreadPool(Swift::EventLoop *loop, int maxthreads) : MAX_THREADS(maxthreads)
+{
 		worker[i] = NULL;
 		freeThreads.push(i);
+	}
 	onWorkCompleted.connect(boost::bind(&ThreadPool::cleandUp, this, _1, _2));
 	onWorkerAvailable.connect(boost::bind(&ThreadPool::scheduleFromQueue, this));
+}
@@ @@ -106,7 +96,7 @@ void ThreadPool::scheduleFromQueue() @@
 		LOG4CXX_INFO(logger, "Worker Available. Creating thread #" << w)
 		Thread *t = requestQueue.front(); requestQueue.pop();
 		t->setThreadID(w);
-		worker[w] = new boost::thread(Worker, t, w, loop);
+		worker[w] = new boost::thread(boost::bind(&ThreadPool::workerBody, this, _1, _2), t, w, loop);
 		updateActiveThreadCount(-1);
+	}
 	criticalregion.unlock();
@@ @@ -119,7 +109,7 @@ void ThreadPool::runAsThread(Thread *t) @@
 	if((w = getFreeThread()) != -1) {
 		LOG4CXX_INFO(logger, "Creating thread #" << w)
 		t->setThreadID(w);
-		worker[w] = new boost::thread(Worker, t, w, loop);
+		worker[w] = new boost::thread(boost::bind(&ThreadPool::workerBody, this, _1, _2), t, w, loop);
 		updateActiveThreadCount(-1);
+	}
 	else {
@@ @@ -128,4 +118,10 @@ void ThreadPool::runAsThread(Thread *t) @@
+	}
+}
 void ThreadPool::workerBody(Thread *t, int wid) {
 	LOG4CXX_INFO(logger, "Starting thread " << wid)
 	t->run();
 	loop->postEvent(boost::bind(&ThreadPool::cleandUp, this, t, wid), SWIFTEN_SHRPTR_NAMESPACE::shared_ptr<Swift::EventOwner>());
+}
+}

0 comments (0 inline, 0 general)