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

@@ @@ -53,21 +53,22 @@ class ThreadPool @@
 	boost::mutex count_lock;
 	boost::mutex pool_lock;
 	boost::mutex criticalregion;
 	Swift::EventLoop *loop;
 	boost::signals2::signal  < void () > onWorkerAvailable;
 	boost::signals2::signal < void () > onWorkerAvailable;
 	public:
 	ThreadPool(Swift::EventLoop *loop, int maxthreads);
 	~ThreadPool();
 	void runAsThread(Thread *t);
 	int getActiveThreadCount();
 	void updateActiveThreadCount(int k);
 	void cleandUp(Thread *, int);
 	void scheduleFromQueue();
 	int getFreeThread();
 	void releaseThread(int i);
 	void workerBody(Thread *t, int wid);
 };
+}

libtransport/ThreadPool.cpp

➞

Show inline comments

@@ @@ -3,32 +3,22 @@ @@
 #include "Swiften/SwiftenCompat.h"
 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)
+{
 	this->loop = loop;
 	activeThreads = 0;
 	worker = (boost::thread **) malloc(sizeof(boost::thread *) * MAX_THREADS);
 	for(int i=0 ; i<MAX_THREADS ; i++) {
 		worker[i] = NULL;
 		freeThreads.push(i);
+	}
 	onWorkCompleted.connect(boost::bind(&ThreadPool::cleandUp, this, _1, _2));
 	onWorkerAvailable.connect(boost::bind(&ThreadPool::scheduleFromQueue, this));
+}
 ThreadPool::~ThreadPool()
+{
 	for(int i=0; i<MAX_THREADS ; i++) {
@@ @@ -103,29 +93,35 @@ void ThreadPool::scheduleFromQueue() @@
 		int  w = getFreeThread();
 		if(w == -1) break;
 		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();
+}
 void ThreadPool::runAsThread(Thread *t)
+{
 	int w;
 	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 {
 		LOG4CXX_INFO(logger, "No workers available! adding to queue.")
 		requestQueue.push(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)