cpp/openttd-patchpack/source Changeset - r28387:de66297ff4e7

Changeset - r28387:de66297ff4e7

Parent rev.

Child rev.

[Not reviewed]

master

0 2 0

Jonathan G Rennison - 11 months ago 2024-01-04 01:02:22
j.g.rennison@gmail.com

Fix: libcurl HTTP thread race at uninit preventing thread exit

2 files changed with 28 insertions and 5 deletions:

src/network/core/http_curl.cpp

src/network/core/http_shared.h

0 comments (0 inline, 0 general)

src/network/core/http_curl.cpp

➞

Show inline comments

@@ @@ -227,14 +227,16 @@ void HttpThread() @@
 			/* No need to be verbose about rate limiting. */
 			Debug(net, (request->callback.cancelled || _http_thread_exit || status_code == HTTP_429_TOO_MANY_REQUESTS) ? 1 : 0, "HTTP request failed: status_code: {}, error: {}", status_code, curl_easy_strerror(res));
 			request->callback.OnFailure();
+		}
 		/* Wait till the callback tells us all data is dequeued. */
 		request->callback.WaitTillEmpty();
 		/* Wait till the callback tells us all data is dequeued, or _http_thread_exit has been set. */
 		request->callback.WaitTillEmptyOrCondition([]() -> bool {
 			return _http_thread_exit;
 		});
+	}
 	curl_easy_cleanup(curl);
+}
 void NetworkHTTPInitialize()
@@ @@ -275,12 +277,17 @@ void NetworkHTTPInitialize() @@
 void NetworkHTTPUninitialize()
+{
 	curl_global_cleanup();
 	_http_thread_exit = true;
 	/* Queues must be cleared (and the queue CV signalled) after _http_thread_exit is set to ensure that the HTTP thread can exit */
 	for (auto &callback : _http_callbacks) {
 		callback->ClearQueue();
+	}
+	{
 		std::lock_guard<std::mutex> lock(_http_mutex);
 		_http_cv.notify_one();
+	}
 	if (_http_thread.joinable()) {

src/network/core/http_shared.h

➞

Show inline comments

@@ @@ -72,19 +72,21 @@ public: @@
 		this->queue.clear();
 		this->queue_cv.notify_all();
+	}
 	/**
 	 * Wait till the queue is dequeued.
 	 * Wait till the queue is dequeued, or a condition is met.
 	 * @param condition Condition functor.
 	 */
 	void WaitTillEmpty()
 	template <typename T>
 	void WaitTillEmptyOrCondition(T condition)
+	{
 		std::unique_lock<std::mutex> lock(this->mutex);
 		while (!queue.empty()) {
+		while (!(queue.empty() || condition())) {
 			this->queue_cv.wait(lock);
+		}
+	}
 	/**
 	 * Check if the queue is empty.
@@ @@ -92,12 +94,26 @@ public: @@
 	bool IsQueueEmpty()
+	{
 		std::lock_guard<std::mutex> lock(this->mutex);
 		return this->queue.empty();
+	}
 	/**
 	 * Clear everything in the queue.
+	 *
 	 * Should be called from the Game Thread.
 	 */
 	void ClearQueue()
+	{
 		std::lock_guard<std::mutex> lock(this->mutex);
 		this->queue.clear();
 		this->queue_cv.notify_all();
+	}
 	HTTPThreadSafeCallback(HTTPCallback *callback) : callback(callback) {}
 	~HTTPThreadSafeCallback()
+	{
 		std::lock_guard<std::mutex> lock(this->mutex);

0 comments (0 inline, 0 general)