diff --git a/indra/aistatemachine/aistatemachine.cpp b/indra/aistatemachine/aistatemachine.cpp index 5352435d2..f5fab22e7 100644 --- a/indra/aistatemachine/aistatemachine.cpp +++ b/indra/aistatemachine/aistatemachine.cpp @@ -82,7 +82,8 @@ void AIStateMachine::setMaxCount(F32 StateMachineMaxTime) void AIStateMachine::run(AIStateMachine* parent, state_type new_parent_state, bool abort_parent, bool on_abort_signal_parent) { - DoutEntering(dc::statemachine, "AIStateMachine::run(" << (void*)parent << ", " << (parent ? parent->state_str(new_parent_state) : "NA") << ", " << abort_parent << ") [" << (void*)this << "]"); + DoutEntering(dc::statemachine, "AIStateMachine::run(" << (void*)parent << ", " << (parent ? parent->state_str(new_parent_state) : "NA") << + ", " << abort_parent << ", " << on_abort_signal_parent << ") [" << (void*)this << "]"); // Must be the first time we're being run, or we must be called from a callback function. llassert(!mParent || mState == bs_callback); llassert(!mCallback || mState == bs_callback); @@ -113,7 +114,7 @@ void AIStateMachine::run(AIStateMachine* parent, state_type new_parent_state, bo // Mark that run() has been called, in case we're being called from a callback function. mState = bs_initialize; - // Set mIdle to false and add statemachine to continued_statemachines. + // Set mIdle to false and add statemachine to continued_statemachines-- or, when boosted, run the statemachine till it finished or goes idle. mSetStateLock.lock(); locked_cont(); } @@ -140,7 +141,7 @@ void AIStateMachine::run(callback_type::signal_type::slot_type const& slot) // Mark that run() has been called, in case we're being called from a callback function. mState = bs_initialize; - // Set mIdle to false and add statemachine to continued_statemachines. + // Set mIdle to false and add statemachine to continued_statemachines-- or, when boosted, run the statemachine till it finished or goes idle. mSetStateLock.lock(); locked_cont(); } @@ -148,9 +149,11 @@ void AIStateMachine::run(callback_type::signal_type::slot_type const& slot) void AIStateMachine::idle(void) { DoutEntering(dc::statemachine, "AIStateMachine::idle() [" << (void*)this << "]"); - llassert(is_main_thread()); + llassert(mInsideMultiplexImpl); + mSetStateLock.lock(); llassert(!mIdle); mIdle = true; + mSetStateLock.unlock(); mSleep = 0; #ifdef SHOW_ASSERT mCalledThreadUnsafeIdle = true; @@ -160,9 +163,12 @@ void AIStateMachine::idle(void) void AIStateMachine::idle(state_type current_run_state) { DoutEntering(dc::statemachine, "AIStateMachine::idle(" << state_str(current_run_state) << ") [" << (void*)this << "]"); - llassert(is_main_thread()); - llassert(!mIdle); + llassert(mInsideMultiplexImpl); // Only boosted state machines can come here as non-main thread because you may only get here from multiplex_impl(). mSetStateLock.lock(); + // If two different threads call multiplex_impl() (boosted state machines) then that should happen serialized: + // only one thread may call multiplex() at any given time. Hence, it is impossible that idle() is called while + // we are already idle. + llassert(!mIdle); // Only go idle if the run state is (still) what we expect it to be. // Otherwise assume that another thread called set_state() and continue running. if (current_run_state == mRunState) @@ -196,6 +202,7 @@ void AIStateMachine::locked_cont(void) mIdle = false; bool not_active = mActive == as_idle; mIdleActive.unlock(); + mBoost = thread_safe_impl(); // mActive is only changed in AIStateMachine::mainloop, by the main-thread, and // here, possibly by any thread. However, after setting mIdle to false above, it // is impossible for any thread to come here, until after the main-thread called @@ -210,8 +217,15 @@ void AIStateMachine::locked_cont(void) // to release mSetStateLock here, with as advantage that if we're not the main- // thread and not_active is true, then the main-thread won't block when it has // a timer running that times out and calls set_state(). + // Finally, if mBoost is true then not_active is not going to be tested and + // mActive is not going to be changed, so then we're done with this lock too. mSetStateLock.unlock(); - if (not_active) + if (mBoost) + { + // Run statemachine until it is finished or idle again. + multiplex(0); + } + else if (not_active) { AIWriteAccess csme_w(sContinuedStateMachinesAndMainloopEnabled); // See above: it is not possible that mActive was changed since not_active @@ -236,9 +250,6 @@ void AIStateMachine::set_state(state_type state) // Stop race condition of multiple threads calling cont() or set_state() here. mSetStateLock.lock(); - // Do not call set_state() unless running. - llassert(mState == bs_run || !is_main_thread()); - // If this function is called from another thread than the main thread, then we have to ignore // it if we're not idle and the state is less than the current state. The main thread must // be able to change the state to anything (also smaller values). Note that that only can work @@ -279,8 +290,11 @@ void AIStateMachine::set_state(state_type state) return; // Ignore. } + // Do not call set_state() from multiplex_impl unless running. + llassert(mState == bs_run || !mInsideMultiplexImpl); + // Do not call idle() when set_state is called from another thread; use idle(state_type) instead. - llassert(!mCalledThreadUnsafeIdle || is_main_thread()); + llassert(!mCalledThreadUnsafeIdle || AIThreadID::in_main_thread()); // Change mRunState to the requested value. if (mRunState != state) @@ -293,7 +307,15 @@ void AIStateMachine::set_state(state_type state) if (mIdle) locked_cont(); // This unlocks mSetStateLock. else + { + // The state was changed while not being idle. + // Note that if thread_safe_impl() returns true now then we don't need to call multiplex() here + // when we ended up here from multiplex(). However, if we end up here as the result of an + // external state change while the state machine is running in the main thread, then we can't + // call multiplex here; it would be too dangerous. In that case we let the main thread pick + // up this state. mSetStateLock.unlock(); + } // If we get here then mIdle is false, so only mRunState can still be changed but we won't // call locked_cont() anymore. When the main thread finally picks up on the state change, @@ -454,6 +476,7 @@ void AIStateMachine::multiplex(U64 current_time) // amount of time has passed. if (mSleep != 0) { + llassert(!mBoost); // Sleeping MUST be done in the main thread: thread_safe_impl() should have returned false. if (mSleep < 0) { if (++mSleep) @@ -475,11 +498,90 @@ void AIStateMachine::multiplex(U64 current_time) { mAborted = false; mState = bs_run; +#ifdef SHOW_ASSERT + mInsideMultiplexImpl = true; +#endif initialize_impl(); +#ifdef SHOW_ASSERT + mInsideMultiplexImpl = false; +#endif if (mAborted || mState != bs_run) + { + mBoost = false; return; + } + mSetStateLock.lock(); + mBoost = thread_safe_impl(); + if (!mBoost && !AIThreadID::in_main_thread()) + { + // Pass running on the main thread. + mIdle = true; + locked_cont(); + return; + } + mSetStateLock.unlock(); + } + if (!mBoost) + { + llassert(AIThreadID::in_main_thread()); +#ifdef SHOW_ASSERT + mInsideMultiplexImpl = true; +#endif + multiplex_impl(); +#ifdef SHOW_ASSERT + mInsideMultiplexImpl = false; +#endif + } + else + { + if (mState != bs_run) + return; + + mSetStateLock.lock(); + llassert(!mIdle); // We should never get here when idle. + + if (!mMultiplexMutex.tryLock()) // Point A + { + // If another thread is already already calling multiplex_impl() then we can't do that too. + // However, instead of blocking we simply return: by getting here it is guaranteed that + // the other thread will do at least one more test of 'mState == bs_run && !mIdle' before + // stopping to run the statemachine. + // The reason for that is that if another thread had mMultiplexMutex at point A, while + // we had mSetStateLock, then the other thread has to be between point B and point C + // because that is the only area where mSetStateLock is unlocked while mMultiplexMutex + // is locked, and the test follows this area. + mSetStateLock.unlock(); + return; + } + + do + { + mSetStateLock.unlock(); // Point B +#ifdef SHOW_ASSERT + mInsideMultiplexImpl = true; +#endif + multiplex_impl(); +#ifdef SHOW_ASSERT + mInsideMultiplexImpl = false; +#endif + mSetStateLock.lock(); // Point C + + // The state might have changed, check if we may still run outside the main thread. + mBoost = thread_safe_impl(); + if (!mBoost && !AIThreadID::in_main_thread()) + { + mMultiplexMutex.unlock(); + // Pass running on the main thread. + mIdle = true; + locked_cont(); + return; + } + } + while (mState == bs_run && !mIdle); + + mMultiplexMutex.unlock(); + mSetStateLock.unlock(); } - multiplex_impl(); } //static @@ -590,6 +692,10 @@ void AIStateMachine::flush(void) for (active_statemachines_type::iterator iter = active_statemachines.begin(); iter != active_statemachines.end(); ++iter) { AIStateMachine& statemachine(iter->statemachine()); +#ifdef SHOW_ASSERT + // Make assertions happy. + statemachine.mInsideMultiplexImpl = true; +#endif if (statemachine.abortable()) { // We can't safely call abort() here for non-running (run() was called, but they weren't initialized yet) statemachines, diff --git a/indra/aistatemachine/aistatemachine.h b/indra/aistatemachine/aistatemachine.h index 9048260ba..6b69f2709 100644 --- a/indra/aistatemachine/aistatemachine.h +++ b/indra/aistatemachine/aistatemachine.h @@ -176,6 +176,7 @@ // Should return a stringified value of run_state. // class AIStateMachine { + protected: //! The type of mState enum base_state_type { bs_initialize, @@ -213,12 +214,15 @@ class AIStateMachine { base_state_type mState; //!< State of the base class. bool mIdle; //!< True if this state machine is not running. bool mAborted; //!< True after calling abort() and before calling run(). - active_type mActive; //!< Whether statemachine is idle, queued to be added to the active list, or already on the active list. - S64 mSleep; //!< Non-zero while the state machine is sleeping. - LLMutex mIdleActive; //!< Used for atomic operations on the pair mIdle / mActive. + bool mBoost; //!< True when not being run by the main thread, but by which ever thread is changing the state. + active_type mActive; //!< Whether statemachine is idle, queued to be added to the active list, or already on the active list (not used for boosted SM). + S64 mSleep; //!< Non-zero while the state machine is sleeping (not used for boosted SM). + LLMutex mIdleActive; //!< Used for atomic operations on the pair mIdle / mActive (not used for boosted SM). + LLMutex mMultiplexMutex; //!< Used for boosted state machines, locked during multiplexing. #ifdef SHOW_ASSERT AIThreadID mContThread; //!< Thread that last called locked_cont(). bool mCalledThreadUnsafeIdle; //!< Set to true when idle() is called. + bool mInsideMultiplexImpl; //!< Set to true while running a boosted state machine (also set while inside initialize_impl). #endif // Callback facilities. @@ -250,10 +254,11 @@ class AIStateMachine { public: //! Create a non-running state machine. - AIStateMachine(void) : mState(bs_initialize), mIdle(true), mAborted(true), mActive(as_idle), mSleep(0), mParent(NULL), mCallback(NULL) + AIStateMachine(void) : mState(bs_initialize), mIdle(true), mAborted(true), mActive(as_idle), mSleep(0), #ifdef SHOW_ASSERT - , mContThread(AIThreadID::none), mCalledThreadUnsafeIdle(false) + mContThread(AIThreadID::none), mCalledThreadUnsafeIdle(false), #endif + mParent(NULL), mCallback(NULL), mRunState(0) { } protected: @@ -374,6 +379,10 @@ class AIStateMachine { //! Return a stringified state, for debugging purposes. char const* state_str(state_type state); + protected: + //! Return mState. + base_state_type base_state(void) const { return mState; } + private: static void add_continued_statemachines(AIReadAccess& csme_r); static void dowork(void); @@ -412,6 +421,10 @@ class AIStateMachine { // Handle cleaning up from initialization (or post abort) state. virtual void finish_impl(void) = 0; + // Determine if it is thread-safe to run the current state in another thread than the main-thread. + // Only called while mSetStateLock is locked, may ONLY access mState and mRunState. + virtual bool thread_safe_impl(void) const { return false; } + // Implemenation of state_str for run states. virtual char const* state_str_impl(state_type run_state) const = 0; }; diff --git a/indra/aistatemachine/aitimer.cpp b/indra/aistatemachine/aitimer.cpp index 5a37a6991..8a909366a 100644 --- a/indra/aistatemachine/aitimer.cpp +++ b/indra/aistatemachine/aitimer.cpp @@ -33,7 +33,8 @@ enum timer_state_type { AITimer_start = AIStateMachine::max_state, - AITimer_expired + AITimer_expired, + AITimer_abort }; char const* AITimer::state_str_impl(state_type run_state) const @@ -42,6 +43,7 @@ char const* AITimer::state_str_impl(state_type run_state) const { AI_CASE_RETURN(AITimer_start); AI_CASE_RETURN(AITimer_expired); + AI_CASE_RETURN(AITimer_abort); } return "UNKNOWN STATE"; } @@ -52,6 +54,12 @@ void AITimer::initialize_impl(void) set_state(AITimer_start); } +void AITimer::request_abort(void) +{ + mFrameTimer.cancel(); + set_state(AITimer_abort); +} + void AITimer::expired(void) { set_state(AITimer_expired); @@ -64,7 +72,7 @@ void AITimer::multiplex_impl(void) case AITimer_start: { mFrameTimer.create(mInterval, boost::bind(&AITimer::expired, this)); - idle(); + idle(AITimer_start); break; } case AITimer_expired: @@ -72,6 +80,11 @@ void AITimer::multiplex_impl(void) finish(); break; } + case AITimer_abort: + { + abort(); + break; + } } } diff --git a/indra/aistatemachine/aitimer.h b/indra/aistatemachine/aitimer.h index c10559429..c4577123f 100644 --- a/indra/aistatemachine/aitimer.h +++ b/indra/aistatemachine/aitimer.h @@ -82,6 +82,11 @@ class AITimer : public AIStateMachine { */ F64 getInterval(void) const { return mInterval; } + /** + * @brief Abort the timer from another thread. + */ + void request_abort(void); + protected: // Call finish() (or abort()), not delete. /*virtual*/ ~AITimer() { DoutEntering(dc::statemachine, "~AITimer() [" << (void*)this << "]"); mFrameTimer.cancel(); } diff --git a/indra/llmessage/aicurleasyrequeststatemachine.cpp b/indra/llmessage/aicurleasyrequeststatemachine.cpp index b9f486d67..ec2056dd2 100644 --- a/indra/llmessage/aicurleasyrequeststatemachine.cpp +++ b/indra/llmessage/aicurleasyrequeststatemachine.cpp @@ -33,26 +33,13 @@ #include "aihttptimeoutpolicy.h" #include "llcontrol.h" -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_removed_after_finished, - AICurlEasyRequestStateMachine_bad_file_descriptor -}; - char const* AICurlEasyRequestStateMachine::state_str_impl(state_type run_state) const { switch(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 +64,12 @@ void AICurlEasyRequestStateMachine::initialize_impl(void) // CURL-THREAD void AICurlEasyRequestStateMachine::added_to_multi_handle(AICurlEasyRequest_wat&) { - set_state(AICurlEasyRequestStateMachine_added); } // CURL-THREAD void AICurlEasyRequestStateMachine::finished(AICurlEasyRequest_wat&) { mFinished = true; - set_state(AICurlEasyRequestStateMachine_finished); } // CURL-THREAD @@ -130,15 +115,11 @@ void AICurlEasyRequestStateMachine::multiplex_impl(void) // 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 // 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) { @@ -151,22 +132,16 @@ void AICurlEasyRequestStateMachine::multiplex_impl(void) } 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(current_state); // 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(AICurlEasyRequestStateMachine_waitAdded); 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. @@ -179,7 +154,6 @@ void AICurlEasyRequestStateMachine::multiplex_impl(void) idle(current_state); // Wait till AICurlEasyRequestStateMachine::removed_from_multi_handle() is called. break; } - case AICurlEasyRequestStateMachine_finished: case AICurlEasyRequestStateMachine_removed_after_finished: { if (!mHandled) @@ -191,7 +165,8 @@ void AICurlEasyRequestStateMachine::multiplex_impl(void) { // Stop the timer. Note that it's the main thread that generates timer events, // so we're certain that there will be no time out anymore if we reach this point. - mTimer->abort(); + // Note that we can't call abort() directly here, as we're a boosted statemachine. + mTimer->request_abort(); } // The request finished and either data or an error code is available. @@ -199,12 +174,6 @@ void AICurlEasyRequestStateMachine::multiplex_impl(void) easy_request_w->processOutput(); } - if (current_state == AICurlEasyRequestStateMachine_finished) - { - idle(current_state); // Wait till AICurlEasyRequestStateMachine::removed_from_multi_handle() is called. - break; - } - // See above. mTimedOut = false; /* Fall-Through */ @@ -264,7 +233,9 @@ void AICurlEasyRequestStateMachine::finish_impl(void) // Note that even if the timer expired, it wasn't deleted because we used AIPersistentTimer; so mTimer is still valid. // Stop the timer, if it's still running. if (!mHandled) - mTimer->abort(); + { + mTimer->request_abort(); + } } // Auto clean up ourselves. kill(); diff --git a/indra/llmessage/aicurleasyrequeststatemachine.h b/indra/llmessage/aicurleasyrequeststatemachine.h index 8a6441874..0b1677b19 100644 --- a/indra/llmessage/aicurleasyrequeststatemachine.h +++ b/indra/llmessage/aicurleasyrequeststatemachine.h @@ -35,6 +35,15 @@ #include "aitimer.h" #include "aicurl.h" +enum curleasyrequeststatemachine_state_type { + AICurlEasyRequestStateMachine_addRequest = AIStateMachine::max_state, + AICurlEasyRequestStateMachine_waitAdded, + 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 +}; + // A curl easy request state machine. // // Before calling cersm.run() initialize the object (cersm) as follows: diff --git a/indra/llmessage/aicurlthread.cpp b/indra/llmessage/aicurlthread.cpp index c732c9226..572c5e4a1 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,20 @@ 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. + + // Stop two threads running the following code at the same time. + if (!mWakeUpMutex.tryLock()) + { + // If another thread is already busy waking up the curl thread, then we don't have to. + 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 +1119,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 +1131,7 @@ void AICurlThread::wakeup_thread(bool stop_thread) } llassert_always(len == 1); #endif + mWakeUpMutex.unlock(); } apr_status_t AICurlThread::join_thread(void) @@ -1247,9 +1259,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 +1324,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 +1336,7 @@ void AICurlThread::run(void) // wakeup_thread() is also called after setting mRunning to false. if (!mRunning) { - mWakeUpMutex.unlock(); + mWakeUpFlagMutex.unlock(); break; } @@ -1400,7 +1412,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..5971f4add 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(); } 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/llmessage/llhttpclient.h b/indra/llmessage/llhttpclient.h index 6142117b0..31f372958 100644 --- a/indra/llmessage/llhttpclient.h +++ b/indra/llmessage/llhttpclient.h @@ -202,6 +202,9 @@ public: // The name of the derived responder object. For debugging purposes. virtual char const* getName(void) const = 0; + // Returning true causes AICurlEasyRequestStateMachine::multiplex_impl to be called from non-main threads. + virtual bool thread_safe_complete(void) const { return false; } + protected: // Derived classes can override this to get the HTML headers that were received, when the message is completed. // Only actually called for classes that implement a needsHeaders() that returns true. diff --git a/indra/llmessage/llurlrequest.cpp b/indra/llmessage/llurlrequest.cpp index 429a93627..c8fb7bd70 100644 --- a/indra/llmessage/llurlrequest.cpp +++ b/indra/llmessage/llurlrequest.cpp @@ -83,6 +83,24 @@ LLURLRequest::LLURLRequest(LLURLRequest::ERequestAction action, std::string cons { } +bool LLURLRequest::thread_safe_impl(void) const +{ + if (base_state() == bs_run && + (mRunState == AICurlEasyRequestStateMachine_removed_after_finished || + mRunState == AICurlEasyRequestStateMachine_addRequest) && + mResponder->thread_safe_complete()) + { + return true; + } + + // AICurlEasyRequestStateMachine::initialize_impl is thread safe because the statemachine + // will only just have been created and no other thread can know of this instance. + if (base_state() == bs_initialize) + return true; + + return false; +} + void LLURLRequest::initialize_impl(void) { // If the header is "Pragma" with no value, the caller intends to diff --git a/indra/llmessage/llurlrequest.h b/indra/llmessage/llurlrequest.h index 9cdb132bb..6f5f3982f 100644 --- a/indra/llmessage/llurlrequest.h +++ b/indra/llmessage/llurlrequest.h @@ -122,6 +122,9 @@ class LLURLRequest : public AICurlEasyRequestStateMachine { protected: // Handle initializing the object. /*virtual*/ void initialize_impl(void); + + // Return true if current state is thread safe. + /*virtual*/ bool thread_safe_impl(void) const; }; #endif // LL_LLURLREQUEST_H 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/lltexturefetch.cpp b/indra/newview/lltexturefetch.cpp index accd96b42..79970338c 100644 --- a/indra/newview/lltexturefetch.cpp +++ b/indra/newview/lltexturefetch.cpp @@ -338,6 +338,8 @@ public: } #endif + /*virtual*/ bool thread_safe_complete(void) const { return true; } + /*virtual*/ void completedRaw(U32 status, const std::string& reason, const LLChannelDescriptors& channels, const buffer_ptr_t& buffer)