From 60ce696e26a9351bb600e71a45d01d37cdf61628 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 1 Feb 2002 15:43:37 +0000 Subject: [PATCH] made wxCondition::Signal() queue the signals generated while there were no waiters and documented this behaviour git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@13956 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775 --- docs/latex/wx/conditn.tex | 59 ++++++++++++- samples/console/console.cpp | 15 ++-- src/unix/threadpsx.cpp | 170 ++++++++++-------------------------- 3 files changed, 111 insertions(+), 133 deletions(-) diff --git a/docs/latex/wx/conditn.tex b/docs/latex/wx/conditn.tex index a58d8b5ff4..88b04eeb28 100644 --- a/docs/latex/wx/conditn.tex +++ b/docs/latex/wx/conditn.tex @@ -11,8 +11,63 @@ perfect because in this particular case it would be much better to just \helpref{Wait()}{wxthreadwait} for the worker thread, but if there are several worker threads it already makes much more sense). -Once the thread(s) are signaled, the condition then resets to the not -signaled state, ready to fire again. +Note that a call to \helpref{Signal()}{wxconditionsignal} may happen before the +other thread calls \helpref{Wait()}{wxconditionwait} but, in marked contrast +with the pthread conditions, this will still work as the missed signals are +queued and \helpref{Wait()}{wxconditionwait} simply returns immediately if +there are ny pending signals. + +However, the calls to \helpref{Broadcast()}{wxconditionbroadcast} are {\bf not} +queued and so it will only wake up the threads already waiting on the +condition. Accordingly, you will probably want to use a mutex to ensure that +the thread(s) you want to be waken up have indeed started to wait before +calling \helpref{Broadcast()}{wxconditionbroadcast}. + +\wxheading{Example} + +This example shows how a main thread may launch a worker thread and wait until +it starts running: + +\begin{verbatim} +class MyWaitingThread : public wxThread +{ +public: + MyWaitingThread(wxCondition *condition) + { + m_condition = condition; + + Create(); + } + + virtual ExitCode Entry() + { + // let the main thread know that we started running + m_condition->Signal(); + + ... do our job ... + + return 0; + } + +private: + wxCondition *m_condition; +}; + +int main() +{ + wxCondition condition; + MyWaitingThread *thread - new MyWaitingThread(&condition); + + thread->Run(); + + // wait until the thread really starts running + condition.Wait(); + + ... + + return 0; +} +\end{verbatim} \wxheading{Derived from} diff --git a/samples/console/console.cpp b/samples/console/console.cpp index 33bafc912b..e67580588a 100644 --- a/samples/console/console.cpp +++ b/samples/console/console.cpp @@ -1117,8 +1117,11 @@ WX_DECLARE_HASH_MAP( short, unsigned, wxIntegerHash, wxIntegerEqual, myTestHashMap3 ); WX_DECLARE_HASH_MAP( unsigned short, unsigned, wxIntegerHash, wxIntegerEqual, myTestHashMap4 ); -WX_DECLARE_HASH_MAP( wxString, wxString, wxStringHash, wxStringEqual, - myStringHashMap ); + +// same as: +// WX_DECLARE_HASH_MAP( wxString, wxString, wxStringHash, wxStringEqual, +// myStringHashMap ); +WX_DECLARE_STRING_HASH_MAP(wxString, myStringHashMap); typedef myStringHashMap::iterator Itor; @@ -1237,7 +1240,7 @@ static void TestHashMap() printf("*** Finished testing wxHashMap ***\n"); } -#endif TEST_HASHMAP +#endif // TEST_HASHMAP // ---------------------------------------------------------------------------- // wxList @@ -4790,7 +4793,7 @@ static void TestThreadConditions() condition.GetId(), gs_cond.GetId()); // create and launch threads - MyWaitingThread *threads[5]; + MyWaitingThread *threads[10]; size_t n; for ( n = 0; n < WXSIZEOF(threads); n++ ) @@ -4823,13 +4826,15 @@ static void TestThreadConditions() wxThread::Sleep(500); -#if 0 +#if 1 // now wake one of them up printf("Main thread: about to signal the condition.\n"); fflush(stdout); condition.Signal(); #endif + wxThread::Sleep(200); + // wake all the (remaining) threads up, so that they can exit printf("Main thread: about to broadcast the condition.\n"); fflush(stdout); diff --git a/src/unix/threadpsx.cpp b/src/unix/threadpsx.cpp index f097dd3af2..960683208e 100644 --- a/src/unix/threadpsx.cpp +++ b/src/unix/threadpsx.cpp @@ -367,39 +367,24 @@ public: void Signal(bool all = FALSE); private: - // the number of Signal() calls queued - // - // changed by Signal(), accessed by Wait() - // - // protected by m_mutexSignalCount - size_t m_nSignalsMissed; - - // protects access to m_nSignalsMissed - pthread_mutex_t m_mutexSignalCount; - - // serializes Broadcast() and/or Signal() calls - // - // TODO: I'm not sure if this is really needed but it shouldn't harm - // neither (except for efficiency condierations). However MSW doesn't - // do this so maybe we shouldn't do it here neither? (VZ) - pthread_mutex_t m_mutexSignal; + // the number of Signal() calls we "missed", i.e. which were done while + // there were no threads to wait for them + size_t m_nQueuedSignals; - // a condition variable must be always used with a mutex and so we maintain - // a list of mutexes - one for each thread that calls Wait(). - // - // access to this list must be protected by m_mutexListContents - wxMutexList m_mutexes; + // counts all pending waiters + size_t m_nWaiters; // the condition itself pthread_cond_t m_condition; - // protects all accesses to m_mutexes list - pthread_mutex_t m_mutexListContents; + // the mutex used with the conditon: it also protects the counters above + pthread_mutex_t m_mutex; }; wxConditionInternal::wxConditionInternal() { - m_nSignalsMissed = 0; + m_nQueuedSignals = + m_nWaiters = 0; if ( pthread_cond_init(&m_condition, (pthread_condattr_t *)NULL) != 0 ) { @@ -407,9 +392,7 @@ wxConditionInternal::wxConditionInternal() wxFAIL_MSG( _T("pthread_cond_init() failed") ); } - if ( pthread_mutex_init(&m_mutexSignalCount, NULL) != 0 || - pthread_mutex_init(&m_mutexListContents, NULL) != 0 || - pthread_mutex_init(&m_mutexSignal, NULL) != 0 ) + if ( pthread_mutex_init(&m_mutex, NULL) != 0 ) { // neither this wxFAIL_MSG( _T("wxCondition: pthread_mutex_init() failed") ); @@ -424,13 +407,7 @@ wxConditionInternal::~wxConditionInternal() "threads are probably still waiting on it?)")); } - // the list of waiters mutexes must be empty by now - wxASSERT_MSG( !m_mutexes.GetFirst(), - _T("deleting condition someone is still waiting on?") ); - - if ( pthread_mutex_destroy( &m_mutexSignalCount ) != 0 || - pthread_mutex_destroy( &m_mutexListContents ) != 0 || - pthread_mutex_destroy( &m_mutexSignal ) != 0 ) + if ( pthread_mutex_destroy( &m_mutex ) != 0 ) { wxLogDebug(_T("Failed to destroy mutex (it is probably locked)")); } @@ -438,53 +415,30 @@ wxConditionInternal::~wxConditionInternal() bool wxConditionInternal::Wait(const timespec* ts) { + MutexLock lock(m_mutex); + + if ( m_nQueuedSignals ) { + m_nQueuedSignals--; + wxLogTrace(TRACE_THREADS, - _T("wxCondition(%08x)::Wait: about to lock missed signal counter"), + _T("wxCondition(%08x)::Wait(): Has been signaled before"), this); - MutexLock lock(m_mutexSignalCount); - - if ( m_nSignalsMissed ) - { - // the condition was signaled before we started to wait, just - // decrease the number of queued signals and return - m_nSignalsMissed--; - - wxLogTrace(TRACE_THREADS, - _T("wxCondition(%08x)::Wait: not waiting at all, count = %u"), - this, m_nSignalsMissed); - - return TRUE; - } + return TRUE; } - // we need to really wait, create a new mutex for this - pthread_mutex_t *mutex = new pthread_mutex_t; - if ( pthread_mutex_init(mutex, (pthread_mutexattr_t *)NULL) != 0 ) - { - // not supposed to happen - wxFAIL_MSG( _T("pthread_mutex_init() failed when starting waiting") ); - } - - // lock the mutex before starting to wait on it - pthread_mutex_lock(mutex); - - // lock the list before modifying it - wxMutexList::Node *mutexNode; - { - MutexLock lockList(m_mutexListContents); - - mutexNode = m_mutexes.Append(mutex); - } + // there are no queued signals, so start really waiting + m_nWaiters++; // calling wait function below unlocks the mutex and Signal() or // Broadcast() will be able to continue to run now if they were // blocking for it in the loop locking all mutexes) wxLogTrace(TRACE_THREADS, _T("wxCondition(%08x)::Wait(): starting to wait"), this); - int err = ts ? pthread_cond_timedwait(&m_condition, mutex, ts) - : pthread_cond_wait(&m_condition, mutex); + + int err = ts ? pthread_cond_timedwait(&m_condition, &m_mutex, ts) + : pthread_cond_wait(&m_condition, &m_mutex); switch ( err ) { case 0: @@ -499,67 +453,31 @@ bool wxConditionInternal::Wait(const timespec* ts) case ETIMEDOUT: case EINTR: + // The condition has not been signaled, so we have to + // decrement the counter manually + --m_nWaiters; + // wait interrupted or timeout elapsed wxLogTrace(TRACE_THREADS, _T("wxCondition(%08x)::Wait(): timeout/intr"), this); } - // delete the mutex we had used for waiting - { - MutexLock lock(m_mutexListContents); - - pthread_mutex_t *m = mutexNode->GetData(); - pthread_mutex_unlock(m); - pthread_mutex_destroy(m); - delete m; - - m_mutexes.DeleteNode(mutexNode); - } - return err == 0; } void wxConditionInternal::Signal(bool all) { // make sure that only one Signal() or Broadcast() is in progress - MutexLock lock(m_mutexSignal); - - // this mutex has to be locked as well, so that during the entire Signal() - // call, no new Wait() is going to wreak havoc (it will block in the very - // beginning on this mutex instead) - MutexLock lockSignalCount(m_mutexSignalCount); - - wxLogTrace(TRACE_THREADS, - _T("wxCondition(%08x)::Signal(): got signal count mutex"), - this); + MutexLock lock(m_mutex); + // Are there any waiters? + if ( m_nWaiters == 0 ) { - MutexLock lockList(m_mutexListContents); - - if ( !m_mutexes.GetFirst() ) - { - // nobody is waiting for us, just remember that the condition was - // signaled and don't do anything else for now - m_nSignalsMissed++; - - wxLogTrace(TRACE_THREADS, - _T("wxCondition(%08x)::Signal(): no waiters, count = %u"), - this, m_nSignalsMissed); - - return; - } - } - - wxLogTrace(TRACE_THREADS, - _T("wxCondition(%08x)::Signal(): acquiring all mutexes"), this); + // No, there are not, so don't signal but keep in mind for the next + // Wait() + m_nQueuedSignals++; - // All mutexes on the list have to be locked. This means that execution of - // Signal() goes on as soon as all pending Wait() calls have called - // pthread_cond_wait() (where the mutex gets unlocked internally) - wxMutexList::Node *node; - for ( node = m_mutexes.GetFirst(); node; node = node->GetNext() ) - { - pthread_mutex_lock(node->GetData()); + return; } // now we can finally signal it @@ -568,21 +486,21 @@ void wxConditionInternal::Signal(bool all) int err = all ? pthread_cond_broadcast(&m_condition) : pthread_cond_signal(&m_condition); - if ( err ) + + if ( all ) { - // shouldn't ever happen - wxFAIL_MSG(_T("pthread_cond_{broadcast|signal}() failed")); + m_nWaiters = 0; } - - // unlock all mutexes so that the threads blocking in their Wait()s could - // continue running - for ( node = m_mutexes.GetFirst(); node; node = node->GetNext() ) + else { - pthread_mutex_unlock(node->GetData()); + --m_nWaiters; } - wxLogTrace(TRACE_THREADS, - _T("wxCondition(%08x)::Signal(): exiting"), this); + if ( err ) + { + // shouldn't ever happen + wxFAIL_MSG(_T("pthread_cond_{broadcast|signal}() failed")); + } } // ---------------------------------------------------------------------------- -- 2.45.2