From 00b223f2a44023eed25dae1e558ba20c57f6490f Mon Sep 17 00:00:00 2001 From: Aleric Inglewood Date: Wed, 20 Mar 2013 22:51:11 +0100 Subject: [PATCH 1/3] Do not request empty folders. Doing this resulted in a 404 on Aditi, and although that was a server bug; it still doesn't seem to make much sense to do the request in the first place. --- .../newview/llinventorymodelbackgroundfetch.cpp | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/indra/newview/llinventorymodelbackgroundfetch.cpp b/indra/newview/llinventorymodelbackgroundfetch.cpp index 3dce0c96a..6b88ab5c5 100644 --- a/indra/newview/llinventorymodelbackgroundfetch.cpp +++ b/indra/newview/llinventorymodelbackgroundfetch.cpp @@ -618,18 +618,7 @@ void LLInventoryModelBackgroundFetch::bulkFetch() if (fetch_info.mIsCategory) { const LLUUID &cat_id = fetch_info.mUUID; - if (cat_id.isNull()) //DEV-17797 - { - LLSD folder_sd; - folder_sd["folder_id"] = LLUUID::null.asString(); - folder_sd["owner_id"] = gAgent.getID(); - folder_sd["sort_order"] = (LLSD::Integer)sort_order; - folder_sd["fetch_folders"] = (LLSD::Boolean)FALSE; - folder_sd["fetch_items"] = (LLSD::Boolean)TRUE; - folder_request_body["folders"].append(folder_sd); - folder_count++; - } - else + if (!cat_id.isNull()) { const LLViewerInventoryCategory* cat = gInventory.getCategory(cat_id); @@ -664,9 +653,9 @@ void LLInventoryModelBackgroundFetch::bulkFetch() } } } + if (fetch_info.mRecursive) + recursive_cats.push_back(cat_id); } - if (fetch_info.mRecursive) - recursive_cats.push_back(cat_id); } else { From 835240fda12aabd908363d5550dbdaaa84dce34c Mon Sep 17 00:00:00 2001 From: Aleric Inglewood Date: Thu, 21 Mar 2013 20:26:01 +0100 Subject: [PATCH 2/3] Fix crash in LLTextureFetch::getWorker upon viewer exit. This is now necessary since the curl thread no longer syncs with the main thread: it is possible that a request finishes after a texture fetch thread was shot down but before curl was stopped, and curl calling BufferedCurlEasyRequest::processOutput while objects that the responder uses were already destructed (most notably LLTextureFetch itself). --- indra/llmessage/aicurl.cpp | 12 ++++++++++++ indra/llmessage/aicurl.h | 9 +++++++-- indra/llmessage/aicurlprivate.h | 5 +++++ indra/llmessage/aicurlthread.cpp | 27 ++++++++++++++++++++------- indra/newview/llappviewer.cpp | 2 ++ 5 files changed, 46 insertions(+), 9 deletions(-) diff --git a/indra/llmessage/aicurl.cpp b/indra/llmessage/aicurl.cpp index 6ea7528bc..3e3e510e1 100644 --- a/indra/llmessage/aicurl.cpp +++ b/indra/llmessage/aicurl.cpp @@ -408,6 +408,16 @@ void initCurl(void) } } +// MAIN-THREAD +void shutdownCurl(void) +{ + using namespace AICurlPrivate; + + DoutEntering(dc::curl, "AICurlInterface::shutdownCurl()"); + + BufferedCurlEasyRequest::shutdown(); +} + // MAIN-THREAD void cleanupCurl(void) { @@ -1293,6 +1303,8 @@ bool CurlEasyRequest::removeFromPerHostQueue(AICurlEasyRequest const& easy_reque static int const HTTP_REDIRECTS_DEFAULT = 10; LLChannelDescriptors const BufferedCurlEasyRequest::sChannels; +LLMutex BufferedCurlEasyRequest::sResponderCallbackMutex; +bool BufferedCurlEasyRequest::sShuttingDown = false; BufferedCurlEasyRequest::BufferedCurlEasyRequest() : mRequestTransferedBytes(0), mResponseTransferedBytes(0), mBufferEventsTarget(NULL), mStatus(HTTP_INTERNAL_ERROR_OTHER) { diff --git a/indra/llmessage/aicurl.h b/indra/llmessage/aicurl.h index 3113e7e18..9244839b7 100644 --- a/indra/llmessage/aicurl.h +++ b/indra/llmessage/aicurl.h @@ -163,8 +163,13 @@ void initCurl(void); // Called once at start of application (from LLAppViewer::initThreads), starts AICurlThread. void startCurlThread(U32 CurlMaxTotalConcurrentConnections, U32 CurlConcurrentConnectionsPerHost, bool NoVerifySSLCert); -// Called once at end of application (from newview/llappviewer.cpp by main thread), -// with purpose to stop curl threads, free curl resources and deinitialize curl. +// Called once at the end of application before terminating other threads (most notably the texture thread workers) +// with the purpose to stop the curl thread from doing any call backs to running responders: the responders sometimes +// access objects that will be shot down when bringing down other threads. +void shutdownCurl(void); + +// Called once at end of application (from newview/llappviewer.cpp by main thread) after all other threads have been terminated +// with the purpose to stop the curl thread, free curl resources and deinitialize curl. void cleanupCurl(void); // Called from indra/newview/llfloaterabout.cpp for the About floater, and diff --git a/indra/llmessage/aicurlprivate.h b/indra/llmessage/aicurlprivate.h index 42912c240..cfd4b5d91 100644 --- a/indra/llmessage/aicurlprivate.h +++ b/indra/llmessage/aicurlprivate.h @@ -386,6 +386,9 @@ class BufferedCurlEasyRequest : public CurlEasyRequest { // Called after removed_from_multi_handle was called. void processOutput(void); + // Called just before shutting down the texture thread, to prevent responder call backs. + static void shutdown(void); + // Do not write more than this amount. //void setBodyLimit(U32 size) { mBodyLimit = size; } @@ -412,6 +415,8 @@ class BufferedCurlEasyRequest : public CurlEasyRequest { public: static LLChannelDescriptors const sChannels; // Channel object for mInput (channel out()) and mOutput (channel in()). + static LLMutex sResponderCallbackMutex; // Locked while calling back any overridden ResponderBase::finished and/or accessing sShuttingDown. + static bool sShuttingDown; // If true, no additional calls to ResponderBase::finished will be made anymore. private: // This class may only be created by constructing a ThreadSafeBufferedCurlEasyRequest. diff --git a/indra/llmessage/aicurlthread.cpp b/indra/llmessage/aicurlthread.cpp index 85be547b2..6234e9425 100644 --- a/indra/llmessage/aicurlthread.cpp +++ b/indra/llmessage/aicurlthread.cpp @@ -1992,20 +1992,33 @@ void BufferedCurlEasyRequest::processOutput(void) { print_diagnostics(code); } - if (mBufferEventsTarget) + sResponderCallbackMutex.lock(); + if (!sShuttingDown) { - // Only the responder registers for these events. - llassert(mBufferEventsTarget == mResponder.get()); - // Allow clients to parse result codes and headers before we attempt to parse - // the body and provide completed/result/error calls. - mBufferEventsTarget->completed_headers(responseCode, responseReason, (code == CURLE_FAILED_INIT) ? NULL : &info); + if (mBufferEventsTarget) + { + // Only the responder registers for these events. + llassert(mBufferEventsTarget == mResponder.get()); + // Allow clients to parse result codes and headers before we attempt to parse + // the body and provide completed/result/error calls. + mBufferEventsTarget->completed_headers(responseCode, responseReason, (code == CURLE_FAILED_INIT) ? NULL : &info); + } + mResponder->finished(code, responseCode, responseReason, sChannels, mOutput); } - mResponder->finished(code, responseCode, responseReason, sChannels, mOutput); + sResponderCallbackMutex.unlock(); mResponder = NULL; resetState(); } +//static +void BufferedCurlEasyRequest::shutdown(void) +{ + sResponderCallbackMutex.lock(); + sShuttingDown = true; + sResponderCallbackMutex.unlock(); +} + void BufferedCurlEasyRequest::received_HTTP_header(void) { if (mBufferEventsTarget) diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index abb906bf7..5e3939d94 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -1758,6 +1758,8 @@ bool LLAppViewer::cleanup() LLViewerMedia::saveCookieFile(); // Stop the plugin read thread if it's running. LLPluginProcessParent::setUseReadThread(false); + // Stop curl responder call backs. + AICurlInterface::shutdownCurl(); llinfos << "Shutting down Threads" << llendflush; From 712c46a74e2a8e7821ae499931b9271f174d03d5 Mon Sep 17 00:00:00 2001 From: Aleric Inglewood Date: Thu, 21 Mar 2013 20:56:21 +0100 Subject: [PATCH 3/3] Add comment with regard to LLSD in the body of pages with an HTTP error code. --- indra/llmessage/llhttpclient.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/indra/llmessage/llhttpclient.cpp b/indra/llmessage/llhttpclient.cpp index 881e7652d..c130938d2 100644 --- a/indra/llmessage/llhttpclient.cpp +++ b/indra/llmessage/llhttpclient.cpp @@ -347,6 +347,12 @@ void LLHTTPClient::ResponderBase::decode_llsd_body(U32 status, std::string const { llwarns << "The server sent us a response with http status " << status << " and LLSD(!) body: \"" << ss.str() << "\"!" << llendl; } + // This is not really an error, and it has been known to happen before. It just normally never happens (at the moment) + // and therefore warrants an investigation. Linden Lab (or other grids) might start to send error messages + // as LLSD in the body in future, but until that happens frequently we might want to leave this assert in. + // Note that an HTTP error code means an error at the transport level in most cases-- so I'm highly suspicious + // when there is any additional information in the body in LLSD format: it is not unlikely to be a server + // bug, where the returned HTTP status code should have been 200 instead. llassert(!server_sent_llsd_with_http_error); } #endif