diff --git a/indra/llmessage/aicurleasyrequeststatemachine.cpp b/indra/llmessage/aicurleasyrequeststatemachine.cpp index a4c42cfe9..ed2bf820d 100644 --- a/indra/llmessage/aicurleasyrequeststatemachine.cpp +++ b/indra/llmessage/aicurleasyrequeststatemachine.cpp @@ -36,10 +36,8 @@ enum curleasyrequeststatemachine_state_type { AICurlEasyRequestStateMachine_addRequest = AIStateMachine::max_state, AICurlEasyRequestStateMachine_waitAdded, - AICurlEasyRequestStateMachine_added, - AICurlEasyRequestStateMachine_timedOut, // This must be smaller than the rest, so they always overrule. - AICurlEasyRequestStateMachine_finished, - AICurlEasyRequestStateMachine_removed, // The removed states must be largest two, so they are never ignored. + AICurlEasyRequestStateMachine_timedOut, // This must be smaller than the rest, so they always overrule. + AICurlEasyRequestStateMachine_removed, // The removed states must be largest two, so they are never ignored. AICurlEasyRequestStateMachine_removed_after_finished, AICurlEasyRequestStateMachine_bad_file_descriptor }; @@ -50,9 +48,7 @@ char const* AICurlEasyRequestStateMachine::state_str_impl(state_type run_state) { AI_CASE_RETURN(AICurlEasyRequestStateMachine_addRequest); AI_CASE_RETURN(AICurlEasyRequestStateMachine_waitAdded); - AI_CASE_RETURN(AICurlEasyRequestStateMachine_added); AI_CASE_RETURN(AICurlEasyRequestStateMachine_timedOut); - AI_CASE_RETURN(AICurlEasyRequestStateMachine_finished); AI_CASE_RETURN(AICurlEasyRequestStateMachine_removed); AI_CASE_RETURN(AICurlEasyRequestStateMachine_removed_after_finished); AI_CASE_RETURN(AICurlEasyRequestStateMachine_bad_file_descriptor); @@ -77,14 +73,12 @@ void AICurlEasyRequestStateMachine::initialize_impl(void) // CURL-THREAD void AICurlEasyRequestStateMachine::added_to_multi_handle(AICurlEasyRequest_wat&) { - advance_state(AICurlEasyRequestStateMachine_added); } // CURL-THREAD void AICurlEasyRequestStateMachine::finished(AICurlEasyRequest_wat&) { mFinished = true; - advance_state(AICurlEasyRequestStateMachine_finished); } // CURL-THREAD @@ -127,15 +121,11 @@ void AICurlEasyRequestStateMachine::multiplex_impl(state_type run_state) // immediately after this call. mAdded = true; mCurlEasyRequest.addRequest(); // This causes the state to be changed, now or later, to - // AICurlEasyRequestStateMachine_added, then - // AICurlEasyRequestStateMachine_finished and then // AICurlEasyRequestStateMachine_removed_after_finished. - // The first two states might be skipped thus, and the state at this point is one of + // The state at this point is thus one of // 1) AICurlEasyRequestStateMachine_waitAdded (idle) - // 2) AICurlEasyRequestStateMachine_added (running) - // 3) AICurlEasyRequestStateMachine_finished (running) - // 4) AICurlEasyRequestStateMachine_removed_after_finished (running) + // 2) AICurlEasyRequestStateMachine_removed_after_finished (running) if (mTotalDelayTimeout > 0.f) { @@ -148,22 +138,16 @@ void AICurlEasyRequestStateMachine::multiplex_impl(state_type run_state) } break; } - case AICurlEasyRequestStateMachine_added: + case AICurlEasyRequestStateMachine_waitAdded: { - // The request was added to the multi handle. This is a no-op, which is good cause - // this state might be skipped anyway ;). - idle(); // Wait for the next event. - - // The state at this point is one of - // 1) AICurlEasyRequestStateMachine_added (idle) - // 2) AICurlEasyRequestStateMachine_finished (running) - // 3) AICurlEasyRequestStateMachine_removed_after_finished (running) + // Nothing to do. + idle(); break; } case AICurlEasyRequestStateMachine_timedOut: { // It is possible that exactly at this point the state changes into - // AICurlEasyRequestStateMachine_finished, with as result that mTimedOut + // AICurlEasyRequestStateMachine_removed_after_finished, with as result that mTimedOut // is set while we will continue with that state. Hence that mTimedOut // is explicitly reset in that state. @@ -176,7 +160,6 @@ void AICurlEasyRequestStateMachine::multiplex_impl(state_type run_state) idle(); // Wait till AICurlEasyRequestStateMachine::removed_from_multi_handle() is called. break; } - case AICurlEasyRequestStateMachine_finished: case AICurlEasyRequestStateMachine_removed_after_finished: { if (!mHandled) @@ -196,12 +179,6 @@ void AICurlEasyRequestStateMachine::multiplex_impl(state_type run_state) easy_request_w->processOutput(); } - if (run_state == AICurlEasyRequestStateMachine_finished) - { - idle(); // Wait till AICurlEasyRequestStateMachine::removed_from_multi_handle() is called. - break; - } - // See above. mTimedOut = false; /* Fall-Through */ diff --git a/indra/llmessage/aicurlthread.cpp b/indra/llmessage/aicurlthread.cpp index c732c9226..4f0ed00b6 100644 --- a/indra/llmessage/aicurlthread.cpp +++ b/indra/llmessage/aicurlthread.cpp @@ -832,8 +832,9 @@ class AICurlThread : public LLThread { public: static AICurlThread* sInstance; - LLMutex mWakeUpMutex; - bool mWakeUpFlag; // Protected by mWakeUpMutex. + LLMutex mWakeUpMutex; // Set while a thread is waking up the curl thread. + LLMutex mWakeUpFlagMutex; // Set when the curl thread is sleeping (in or about to enter select()). + bool mWakeUpFlag; // Protected by mWakeUpFlagMutex. public: // MAIN-THREAD @@ -1067,11 +1068,10 @@ void AICurlThread::cleanup_wakeup_fds(void) #endif } -// MAIN-THREAD +// OTHER THREADS void AICurlThread::wakeup_thread(bool stop_thread) { DoutEntering(dc::curl, "AICurlThread::wakeup_thread"); - llassert(is_main_thread()); // If we are already exiting the viewer then return immediately. if (!mRunning) @@ -1079,12 +1079,29 @@ void AICurlThread::wakeup_thread(bool stop_thread) // Last time we are run? if (stop_thread) - mRunning = false; + mRunning = false; // Thread-safe because all other threads were already stopped. + + // Note, we do not want this function to be blocking the calling thread; therefore we only use tryLock()s. + + // Stop two threads running the following code concurrently. + if (!mWakeUpMutex.tryLock()) + { + // If we failed to obtain mWakeUpMutex then another thread is (or was) in AICurlThread::wakeup_thread, + // or curl was holding the lock for a micro second at the start of process_commands. + // In the first case, curl might or might not yet have been woken up because of that, but if it was + // then it could not have started processing the commands yet, because it needs to obtain mWakeUpMutex + // between being woken up and processing the commands. + // Either way, the command that this thread called this function for was already in the queue (it's + // added before this function is called) but the command(s) that another thread called this function + // for were not processed yet. Hence, it's safe to exit here as our command(s) will be processed too. + return; + } // Try if curl thread is still awake and if so, pass the new commands directly. - if (mWakeUpMutex.tryLock()) + if (mWakeUpFlagMutex.tryLock()) { mWakeUpFlag = true; + mWakeUpFlagMutex.unlock(); mWakeUpMutex.unlock(); return; } @@ -1111,7 +1128,10 @@ void AICurlThread::wakeup_thread(bool stop_thread) { len = write(mWakeUpFd_in, "!", 1); if (len == -1 && errno == EAGAIN) + { + mWakeUpMutex.unlock(); return; // Unread characters are still in the pipe, so no need to add more. + } } while(len == -1 && errno == EINTR); if (len == -1) @@ -1120,6 +1140,12 @@ void AICurlThread::wakeup_thread(bool stop_thread) } llassert_always(len == 1); #endif + + // Release the lock here and not sooner, for the sole purpose of making sure + // that not two threads execute the above code concurrently. If the above code + // is thread-safe (maybe it is?) then we could release this lock arbitrarily + // sooner indeed - or even not lock it at all. + mWakeUpMutex.unlock(); } apr_status_t AICurlThread::join_thread(void) @@ -1239,6 +1265,16 @@ void AICurlThread::process_commands(AICurlMultiHandle_wat const& multi_handle_w) { DoutEntering(dc::curl, "AICurlThread::process_commands(void)"); + // Block here until the thread that woke us up released mWakeUpMutex. + // This is necessary to make sure that a third thread added commands + // too then either it will signal us later, or we process those commands + // now, too. + mWakeUpMutex.lock(); + // Note that if at THIS point another thread tries to obtain mWakeUpMutex in AICurlThread::wakeup_thread + // and fails, it is ok that it leaves that function without waking us up too: we're awake and + // about to process any commands! + mWakeUpMutex.unlock(); + // If we get here then the main thread called wakeup_thread() recently. for(;;) { @@ -1247,9 +1283,9 @@ void AICurlThread::process_commands(AICurlMultiHandle_wat const& multi_handle_w) command_queue_wat command_queue_w(command_queue); if (command_queue_w->empty()) { - mWakeUpMutex.lock(); + mWakeUpFlagMutex.lock(); mWakeUpFlag = false; - mWakeUpMutex.unlock(); + mWakeUpFlagMutex.unlock(); break; } // Move the next command from the queue into command_being_processed. @@ -1312,10 +1348,10 @@ void AICurlThread::run(void) // Process every command in command_queue before filling the fd_set passed to select(). for(;;) { - mWakeUpMutex.lock(); + mWakeUpFlagMutex.lock(); if (mWakeUpFlag) { - mWakeUpMutex.unlock(); + mWakeUpFlagMutex.unlock(); process_commands(multi_handle_w); continue; } @@ -1324,7 +1360,7 @@ void AICurlThread::run(void) // wakeup_thread() is also called after setting mRunning to false. if (!mRunning) { - mWakeUpMutex.unlock(); + mWakeUpFlagMutex.unlock(); break; } @@ -1400,7 +1436,7 @@ void AICurlThread::run(void) #endif #endif ready = select(nfds, read_fd_set, write_fd_set, NULL, &timeout); - mWakeUpMutex.unlock(); + mWakeUpFlagMutex.unlock(); #ifdef CWDEBUG #ifdef DEBUG_CURLIO Dout(dc::finish|cond_error_cf(ready == -1), ready); diff --git a/indra/llmessage/aihttptimeout.cpp b/indra/llmessage/aihttptimeout.cpp index 499921a2e..d642cf2ec 100644 --- a/indra/llmessage/aihttptimeout.cpp +++ b/indra/llmessage/aihttptimeout.cpp @@ -84,6 +84,11 @@ public: #include "aihttptimeout.h" +// If this is set, treat dc::curlio as off in the assertion below. +#if defined(CWDEBUG) || defined(DEBUG_CURLIO) +bool gCurlIo; +#endif + namespace AICurlPrivate { namespace curlthread { @@ -169,7 +174,7 @@ bool HTTPTimeout::data_received(size_t n/*,*/ // using CURLOPT_DEBUGFUNCTION. Note that mDebugIsHeadOrGetMethod is only valid when the debug channel 'curlio' is on, // because it is set in the debug callback function. // This is also normal if we received a HTTP header with an error status, since that can interrupt our upload. - Debug(llassert(upload_error_status || AICurlEasyRequest_wat(*mLockObj)->mDebugIsHeadOrGetMethod || !dc::curlio.is_on())); + Debug(llassert(upload_error_status || AICurlEasyRequest_wat(*mLockObj)->mDebugIsHeadOrGetMethod || !dc::curlio.is_on() || gCurlIo)); // 'Upload finished' detection failed, generate it now. upload_finished(); } @@ -327,7 +332,7 @@ bool HTTPTimeout::lowspeed(size_t bytes) llassert_always(bucket < low_speed_time); total_bytes -= mBuckets[bucket]; // Empty this bucket. } - while(total_bytes >= 1); // Use 1 here instead of mintotalbytes, to test that total_bytes indeed always reaches zero. + while(total_bytes >= mintotalbytes); } // If this function isn't called again within max_stall_time seconds, we stalled. mStalled = sClockCount + max_stall_time / sClockWidth; diff --git a/indra/llmessage/aihttptimeout.h b/indra/llmessage/aihttptimeout.h index ab7895a3b..5fe876f79 100644 --- a/indra/llmessage/aihttptimeout.h +++ b/indra/llmessage/aihttptimeout.h @@ -133,5 +133,9 @@ class HTTPTimeout : public LLRefCount { } // namespace curlthread } // namespace AICurlPrivate +#if defined(CWDEBUG) || defined(DEBUG_CURLIO) +extern bool gCurlIo; +#endif + #endif diff --git a/indra/newview/linux_tools/launch_url.sh b/indra/newview/linux_tools/launch_url.sh index 404ea36f2..c528ee762 100755 --- a/indra/newview/linux_tools/launch_url.sh +++ b/indra/newview/linux_tools/launch_url.sh @@ -45,6 +45,18 @@ if [ ! -z "$XBROWSER" ]; then echo "$0: Trying some others..." fi +# Launcher for any desktop. +if which xdg-open >/dev/null; then + xdg-open "$URL" + case $? in + 0) exit ;; + 1) echo "xdg-open: Error in command line syntax." ;; + 2) echo "xdg-open: One of the files passed on the command line did not exist." ;; + 3) echo "xdg-open: A required tool could not be found." ;; + 4) echo "xdg-open: The action failed." ;; + esac +fi + # Launcher the default GNOME browser. if [ ! -z "$GNOME_DESKTOP_SESSION_ID" ] && which gnome-open >/dev/null; then gnome-open "$URL" & diff --git a/indra/newview/llstartup.cpp b/indra/newview/llstartup.cpp index 1d3361f82..0936422e6 100644 --- a/indra/newview/llstartup.cpp +++ b/indra/newview/llstartup.cpp @@ -260,10 +260,6 @@ extern S32 gStartImageHeight; // local globals // -#if defined(CWDEBUG) || defined(DEBUG_CURLIO) -static bool gCurlIo; -#endif - static LLHost gAgentSimHost; static BOOL gSkipOptionalUpdate = FALSE; diff --git a/indra/newview/llvoavatar.cpp b/indra/newview/llvoavatar.cpp index fee8c5253..8a949e275 100644 --- a/indra/newview/llvoavatar.cpp +++ b/indra/newview/llvoavatar.cpp @@ -7419,7 +7419,11 @@ void LLVOAvatar::processAvatarAppearance( LLMessageSystem* mesgsys ) ESex old_sex = getSex(); LLTEContents tec; - parseTEMessage(mesgsys, _PREHASH_ObjectData, -1, tec); + if (!parseTEMessage(mesgsys, _PREHASH_ObjectData, -1, tec)) + { + llwarns << avString() << "Failed to parse ObjectData" << llendl; + return; + } U8 appearance_version = 0; S32 this_update_cof_version = LLViewerInventoryCategory::VERSION_UNKNOWN;