From 4744839a088c474cd3845667b87367e27641f448 Mon Sep 17 00:00:00 2001 From: Aleric Inglewood Date: Sun, 23 Sep 2012 17:31:41 +0200 Subject: [PATCH] Bug fix for curl request queue of last commit. In debug mode an assertion was triggered when a queued request was being removed by the main thread; and rightfully so: we should remove such request from the queue in that case. --- indra/llmessage/aicurl.h | 4 ++++ indra/llmessage/aicurlthread.cpp | 30 +++++++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/indra/llmessage/aicurl.h b/indra/llmessage/aicurl.h index 2402da224..7e1f7c78d 100644 --- a/indra/llmessage/aicurl.h +++ b/indra/llmessage/aicurl.h @@ -310,6 +310,10 @@ class AICurlEasyRequest { // destruct it while another thread still needs it, concurrent or not. AICurlEasyRequest& operator=(AICurlEasyRequest const&) { return *this; } + public: + // Instead of assignment, it might be helpful to use swap. + void swap(AICurlEasyRequest& cer) { mCurlEasyRequest.swap(cer.mCurlEasyRequest); } + public: // The more exotic member functions of this class, to deal with passing this class // as CURLOPT_PRIVATE pointer to a curl handle and afterwards restore it. diff --git a/indra/llmessage/aicurlthread.cpp b/indra/llmessage/aicurlthread.cpp index a75b1f0a0..c29092271 100644 --- a/indra/llmessage/aicurlthread.cpp +++ b/indra/llmessage/aicurlthread.cpp @@ -1461,13 +1461,41 @@ void MultiHandle::add_easy_request(AICurlEasyRequest const& easy_request) } } mQueuedRequests.push_back(easy_request); +#ifdef SHOW_ASSERT + // Not active yet, but it's no longer an error if next we try to remove the request. + AICurlEasyRequest_wat(*easy_request)->mRemovedPerCommand = false; +#endif } CURLMcode MultiHandle::remove_easy_request(AICurlEasyRequest const& easy_request, bool as_per_command) { addedEasyRequests_type::iterator iter = mAddedEasyRequests.find(easy_request); if (iter == mAddedEasyRequests.end()) - return (CURLMcode)-2; // Was already removed before. + { + // The request could be queued. + std::deque::iterator const end = mQueuedRequests.end(); + std::deque::iterator cur = std::find(mQueuedRequests.begin(), end, easy_request); + if (cur != end) + { + // We can't use erase because that uses assignment to move elements, which is private because it isn't thread-safe for AICurlEasyRequest. + // Therefore, move the element that we found to the back with swap (could just swap with the end immediately, + // but I don't want to break the order in which requests where added). Swap is also not thread-safe, but OK here + // because it only touches the AICurlEasyRequest objects in the deque, and the deque is protected by the + // lock on MultiHandle. + std::deque::iterator prev = cur; + while (++cur != end) + { + prev->swap(*cur); + prev = cur; + } +#ifdef SHOW_ASSERT + // Now a second remove command would be an error again. + AICurlEasyRequest_wat(**prev)->mRemovedPerCommand = true; +#endif + mQueuedRequests.pop_back(); + } + return (CURLMcode)-2; // Was already removed before, or never added (queued). + } CURLMcode res; { AICurlEasyRequest_wat curl_easy_request_w(**iter);