diff --git a/indra/llmessage/aicurl.cpp b/indra/llmessage/aicurl.cpp index 06e13e0f0..427721dbd 100644 --- a/indra/llmessage/aicurl.cpp +++ b/indra/llmessage/aicurl.cpp @@ -1036,7 +1036,7 @@ void CurlEasyRequest::finalizeRequest(std::string const& url, AIHTTPTimeoutPolic { DoutCurlEntering("CurlEasyRequest::finalizeRequest(\"" << url << "\", " << policy.name() << ", " << (void*)state_machine << ")"); llassert(!mTimeoutPolicy); // May only call finalizeRequest once! - mResult = CURLE_FAILED_INIT; // General error code; the final result code is stored here by MultiHandle::check_run_count when msg is CURLMSG_DONE. + mResult = CURLE_FAILED_INIT; // General error code; the final result code is stored here by MultiHandle::check_msg_queue when msg is CURLMSG_DONE. #ifdef SHOW_ASSERT // Do a sanity check on the headers. int content_type_count = 0; @@ -1063,13 +1063,13 @@ void CurlEasyRequest::finalizeRequest(std::string const& url, AIHTTPTimeoutPolic // will be used if the object is deleted [In fact, since this is finalizeRequest() and not addRequest(), // incrementing the reference counter would be wrong: if addRequest() is never called then the object is // destroyed shortly after and this pointer is never even used.] - // This pointer is used in MultiHandle::check_run_count, which means that addRequest() was called and + // This pointer is used in MultiHandle::check_msg_queue, which means that addRequest() was called and // the reference counter was increased and the object is being kept alive, see the comments above // command_queue in aicurlthread.cpp. In fact, this object survived until MultiHandle::add_easy_request // was called and is kept alive by MultiHandle::mAddedEasyRequests. The only way to get deleted after // that is when MultiHandle::remove_easy_request is called, which first removes the easy handle from // the multi handle. So that it's (hopefully) no longer possible that info_read() in - // MultiHandle::check_run_count returns this easy handle, after the object is destroyed by deleting + // MultiHandle::check_msg_queue returns this easy handle, after the object is destroyed by deleting // it from MultiHandle::mAddedEasyRequests. setopt(CURLOPT_PRIVATE, get_lockobj()); } diff --git a/indra/llmessage/aicurl.h b/indra/llmessage/aicurl.h index 4424e4299..c5e27db74 100644 --- a/indra/llmessage/aicurl.h +++ b/indra/llmessage/aicurl.h @@ -273,7 +273,7 @@ class AICurlEasyRequest { // it's not thread-safe in itself. AICurlEasyRequest(AICurlPrivate::BufferedCurlEasyRequestPtr const& ptr) : mBufferedCurlEasyRequest(ptr) { } - // This one is obviously dangerous. It's for use only in MultiHandle::check_run_count. + // This one is obviously dangerous. It's for use only in MultiHandle::check_msg_queue. // See also the long comment in BufferedCurlEasyRequest::finalizeRequest with regard to CURLOPT_PRIVATE. explicit AICurlEasyRequest(AICurlPrivate::ThreadSafeBufferedCurlEasyRequest* ptr) : mBufferedCurlEasyRequest(ptr) { } }; diff --git a/indra/llmessage/aicurlprivate.h b/indra/llmessage/aicurlprivate.h index 9fb62ca15..8c8bc3621 100644 --- a/indra/llmessage/aicurlprivate.h +++ b/indra/llmessage/aicurlprivate.h @@ -357,7 +357,7 @@ class CurlEasyRequest : public CurlEasyHandle { // Called by in case of an error. void print_diagnostics(CURLcode code); - // Called by MultiHandle::check_run_count() to fill info with the transfer info. + // Called by MultiHandle::check_msg_queue() to fill info with the transfer info. void getTransferInfo(AITransferInfo* info); // If result != CURLE_FAILED_INIT then also info was filled. diff --git a/indra/llmessage/aicurlthread.cpp b/indra/llmessage/aicurlthread.cpp index 91744ce23..8d3cab884 100644 --- a/indra/llmessage/aicurlthread.cpp +++ b/indra/llmessage/aicurlthread.cpp @@ -1411,7 +1411,7 @@ void AICurlThread::run(void) // that libcurl removed file descriptors which we subsequently // didn't handle. } - multi_handle_w->check_run_count(); + multi_handle_w->check_msg_queue(); } } AICurlMultiHandle::destroyInstance(); @@ -1420,7 +1420,7 @@ void AICurlThread::run(void) //----------------------------------------------------------------------------- // MultiHandle -MultiHandle::MultiHandle(void) : mRunningHandles(0), mTimeout(-1), mReadPollSet(NULL), mWritePollSet(NULL) +MultiHandle::MultiHandle(void) : mTimeout(-1), mReadPollSet(NULL), mWritePollSet(NULL) { mReadPollSet = new PollSet; mWritePollSet = new PollSet; @@ -1504,12 +1504,14 @@ int MultiHandle::timer_callback(CURLM* multi, long timeout_ms, void* userp) CURLMcode MultiHandle::socket_action(curl_socket_t sockfd, int ev_bitmask) { + int running_handles; CURLMcode res; do { - res = check_multi_code(curl_multi_socket_action(mMultiHandle, sockfd, ev_bitmask, &mRunningHandles)); + res = check_multi_code(curl_multi_socket_action(mMultiHandle, sockfd, ev_bitmask, &running_handles)); } while(res == CURLM_CALL_MULTI_PERFORM); + llassert(mAddedEasyRequests.size() >= (size_t)running_handles); return res; } @@ -1614,42 +1616,38 @@ CURLMcode MultiHandle::remove_easy_request(addedEasyRequests_type::iterator cons return res; } -void MultiHandle::check_run_count(void) +void MultiHandle::check_msg_queue(void) { - llassert(mAddedEasyRequests.size() >= (size_t)mRunningHandles); - if (mAddedEasyRequests.size() - (size_t)mRunningHandles > 0) // There is no need to do this when all easy handles are accounted for. + CURLMsg const* msg; + int msgs_left; + while ((msg = info_read(&msgs_left))) { - CURLMsg const* msg; - int msgs_left; - while ((msg = info_read(&msgs_left))) + if (msg->msg == CURLMSG_DONE) { - if (msg->msg == CURLMSG_DONE) + CURL* easy = msg->easy_handle; + ThreadSafeBufferedCurlEasyRequest* ptr; + CURLcode rese = curl_easy_getinfo(easy, CURLINFO_PRIVATE, &ptr); + llassert_always(rese == CURLE_OK); + AICurlEasyRequest easy_request(ptr); + llassert(*AICurlEasyRequest_wat(*easy_request) == easy); + // Store result and trigger events for the easy request. + finish_easy_request(easy_request, msg->data.result); + // This invalidates msg, but not easy_request. + CURLMcode res = remove_easy_request(easy_request); + // This should hold, I think, because the handles are obviously ok and + // the only error we could get is when remove_easy_request() was already + // called before (by this thread); but if that was the case then the easy + // handle should not have been be returned by info_read()... + llassert(res == CURLM_OK); + // Nevertheless, if it was already removed then just ignore it. + if (res == CURLM_OK) { - CURL* easy = msg->easy_handle; - ThreadSafeBufferedCurlEasyRequest* ptr; - CURLcode rese = curl_easy_getinfo(easy, CURLINFO_PRIVATE, &ptr); - llassert_always(rese == CURLE_OK); - AICurlEasyRequest easy_request(ptr); - llassert(*AICurlEasyRequest_wat(*easy_request) == easy); - // Store result and trigger events for the easy request. - finish_easy_request(easy_request, msg->data.result); - // This invalidates msg, but not easy_request. - CURLMcode res = remove_easy_request(easy_request); - // This should hold, I think, because the handles are obviously ok and - // the only error we could get is when remove_easy_request() was already - // called before (by this thread); but if that was the case then the easy - // handle should not have been be returned by info_read()... - llassert(res == CURLM_OK); - // Nevertheless, if it was already removed then just ignore it. - if (res == CURLM_OK) - { - } - else if (res == -2) - { - llwarns << "Curl easy handle returned by curl_multi_info_read() that is not (anymore) in MultiHandle::mAddedEasyRequests!?!" << llendl; - } - // Destruction of easy_request at this point, causes the CurlEasyRequest to be deleted. } + else if (res == -2) + { + llwarns << "Curl easy handle returned by curl_multi_info_read() that is not (anymore) in MultiHandle::mAddedEasyRequests!?!" << llendl; + } + // Destruction of easy_request at this point, causes the CurlEasyRequest to be deleted. } } } diff --git a/indra/llmessage/aicurlthread.h b/indra/llmessage/aicurlthread.h index 1ca677990..32ebad685 100644 --- a/indra/llmessage/aicurlthread.h +++ b/indra/llmessage/aicurlthread.h @@ -75,7 +75,6 @@ class MultiHandle : public CurlMultiHandle private: typedef std::set addedEasyRequests_type; addedEasyRequests_type mAddedEasyRequests; // All easy requests currently added to the multi handle. - int mRunningHandles; // The last value returned by curl_multi_socket_action. long mTimeout; // The last timeout in ms as set by the callback CURLMOPT_TIMERFUNCTION. private: @@ -89,14 +88,11 @@ class MultiHandle : public CurlMultiHandle static int timer_callback(CURLM* multi, long timeout_ms, void* userp); public: - // Returns the number of active easy handles as reported by the last call to curl_multi_socket_action. - int getRunningHandles(void) const { return mRunningHandles; } - // Returns how long to wait for socket action before calling socket_action(CURL_SOCKET_TIMEOUT, 0), in ms. int getTimeout(void) const { return mTimeout; } // This is called before sleeping, after calling (one or more times) socket_action. - void check_run_count(void); + void check_msg_queue(void); // Called from the main loop every time select() timed out. void handle_stalls(void);