summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStephan Bergmann <sbergman@redhat.com>2012-05-23 09:42:37 +0200
committerCaolán McNamara <caolanm@redhat.com>2012-05-24 07:52:29 +0100
commitb7721ccca2f76c52dd6c6bb9b8f673fb276fea52 (patch)
tree2d25739bc6bd4c46bdf2c0fb48b2548255772c52
parentde4b790fa928f92bf40b2e2b2c2acb622f9ebd8b (diff)
Better fix for ThreadPool/ORequestThread life cycle
This is a follow up to d015384e1d98fe77fd59339044f58efb1ab9fb25 "Fixed ThreadPool (and dependent ORequestThread) life cycle" that still had some problems: * First, if Bridge::terminate was first entered from the reader or writer thread, it would not join on that thread, so that thread could still be running during exit. That has been addressed by giving Bridge::dispose new semantics: It waits until both Bridge::terminate has completed (even if that was called from a different thread) and all spawned threads (reader, writer, ORequestThread workers) have been joined. (This implies that Bridge::dispose must not be called from such a thread, to avoid deadlock.) * Second, if Bridge::terminate was first entered from an ORequestThread, the call to uno_threadpool_dispose(0) to join on all such worker threads could deadlock. That has been addressed by making the last call to uno_threadpool_destroy wait to join on all worker threads, and by calling uno_threadpool_destroy only from the final Bridge::terminate (from Bridge::dispose), to avoid deadlock. (The special semantics of uno_threadpool_dispose(0) are no longer needed and have been removed, as they conflicted with the fix for the third problem below.) * Third, once uno_threadpool_destroy had called uno_threadpool_dispose(0), the ThreadAdmin singleton had been disposed, so no new remote bridges could successfully be created afterwards. That has been addressed by making ThreadAdmin a member of ThreadPool, and making (only) those uno_ThreadPool handles with overlapping life spans share one ThreadPool instance (which thus is no longer a singleton, either). Additionally, ORequestThread has been made more robust (in the style of salhelper::Thread) to avoid races. (cherry picked from commit 2fa2660b55a34a5780f9ea8dbbbe92d05dc9a818) Conflicts: binaryurp/source/bridge.cxx cppu/Library_cppu.mk cppu/source/threadpool/threadpool.cxx Change-Id: I2cbd1b3f9aecc1bf4649e482d2c22b33b471788f Signed-off-by: Caolán McNamara <caolanm@redhat.com>
-rw-r--r--binaryurp/source/bridge.cxx205
-rw-r--r--binaryurp/source/bridge.hxx25
-rw-r--r--binaryurp/source/bridgefactory.cxx2
-rw-r--r--binaryurp/source/incomingrequest.cxx2
-rw-r--r--binaryurp/source/reader.cxx2
-rw-r--r--binaryurp/source/writer.cxx2
-rw-r--r--cppu/inc/uno/threadpool.h8
-rw-r--r--cppu/source/threadpool/thread.cxx184
-rw-r--r--cppu/source/threadpool/thread.hxx52
-rw-r--r--cppu/source/threadpool/threadpool.cxx124
-rw-r--r--cppu/source/threadpool/threadpool.hxx43
-rw-r--r--cppu/util/makefile.mk2
-rw-r--r--cpputools/source/unoexe/unoexe.cxx4
13 files changed, 372 insertions, 283 deletions
diff --git a/binaryurp/source/bridge.cxx b/binaryurp/source/bridge.cxx
index f591fe0e9e43..13987fe3bd51 100644
--- a/binaryurp/source/bridge.cxx
+++ b/binaryurp/source/bridge.cxx
@@ -107,7 +107,5 @@ extern "C" void SAL_CALL freeProxyCallback(uno_ExtEnvironment *, void * pProxy)
-void joinThread(osl::Thread * thread) {
+bool isThread(osl::Thread * thread) {
assert(thread != 0);
- if (thread->getIdentifier() != osl::Thread::getCurrentIdentifier()) {
- thread->join();
- }
+ return osl::Thread::getCurrentIdentifier() == thread->getIdentifier();
}
@@ -215,4 +213,4 @@ Bridge::Bridge(
"com.sun.star.bridge.XProtocolProperties::commitChange"))),
- threadPool_(0), currentContextMode_(false), proxies_(0), calls_(0),
- normalCall_(false), activeCalls_(0), terminated_(false),
+ state_(STATE_INITIAL), threadPool_(0), currentContextMode_(false),
+ proxies_(0), calls_(0), normalCall_(false), activeCalls_(0),
mode_(MODE_REQUESTED)
@@ -240,3 +238,5 @@ void Bridge::start() {
osl::MutexGuard g(mutex_);
- assert(threadPool_ == 0 && !writer_.is() && !reader_.is());
+ assert(
+ state_ == STATE_INITIAL && threadPool_ == 0 && !writer_.is() &&
+ !reader_.is());
threadPool_ = uno_threadpool_create();
@@ -245,2 +245,3 @@ void Bridge::start() {
writer_ = w;
+ state_ = STATE_STARTED;
}
@@ -256,56 +257,113 @@ void Bridge::start() {
-void Bridge::terminate() {
+void Bridge::terminate(bool final) {
uno_ThreadPool tp;
- rtl::Reference< Reader > r;
- rtl::Reference< Writer > w;
- Listeners ls;
+ // Make sure function-local variables (Stubs s, etc.) are destroyed before
+ // the final uno_threadpool_destroy/threadPool_ = 0:
{
- osl::MutexGuard g(mutex_);
- if (terminated_) {
- return;
+ rtl::Reference< Reader > r;
+ rtl::Reference< Writer > w;
+ bool joinW;
+ Listeners ls;
+ {
+ osl::ClearableMutexGuard g(mutex_);
+ switch (state_) {
+ case STATE_INITIAL: // via ~Bridge -> dispose -> terminate
+ case STATE_FINAL:
+ return;
+ case STATE_STARTED:
+ break;
+ case STATE_TERMINATED:
+ if (final) {
+ g.clear();
+ terminated_.wait();
+ {
+ osl::MutexGuard g2(mutex_);
+ tp = threadPool_;
+ threadPool_ = 0;
+ assert(!(reader_.is() && isThread(reader_.get())));
+ std::swap(reader_, r);
+ assert(!(writer_.is() && isThread(writer_.get())));
+ std::swap(writer_, w);
+ state_ = STATE_FINAL;
+ }
+ assert(!(r.is() && w.is()));
+ if (r.is()) {
+ r->join();
+ } else if (w.is()) {
+ w->join();
+ }
+ if (tp != 0) {
+ uno_threadpool_destroy(tp);
+ }
+ }
+ return;
+ }
+ tp = threadPool_;
+ assert(!(final && isThread(reader_.get())));
+ if (!isThread(reader_.get())) {
+ std::swap(reader_, r);
+ }
+ w = writer_;
+ joinW = !isThread(writer_.get());
+ assert(!final || joinW);
+ if (joinW) {
+ writer_.clear();
+ }
+ ls.swap(listeners_);
+ state_ = final ? STATE_FINAL : STATE_TERMINATED;
+ }
+ try {
+ connection_->close();
+ } catch (const css::io::IOException & e) {
+ SAL_INFO("binaryurp", "caught IO exception '" << e.Message << '\'');
+ }
+ assert(w.is());
+ w->stop();
+ if (r.is()) {
+ r->join();
+ }
+ if (joinW) {
+ w->join();
+ }
+ assert(tp != 0);
+ uno_threadpool_dispose(tp);
+ Stubs s;
+ {
+ osl::MutexGuard g(mutex_);
+ s.swap(stubs_);
+ }
+ for (Stubs::iterator i(s.begin()); i != s.end(); ++i) {
+ for (Stub::iterator j(i->second.begin()); j != i->second.end(); ++j)
+ {
+ SAL_INFO(
+ "binaryurp",
+ "stub '" << i->first << "', '" << toString(j->first)
+ << "' still mapped at Bridge::terminate");
+ binaryUno_.get()->pExtEnv->revokeInterface(
+ binaryUno_.get()->pExtEnv, j->second.object.get());
+ }
+ }
+ factory_->removeBridge(this);
+ for (Listeners::iterator i(ls.begin()); i != ls.end(); ++i) {
+ try {
+ (*i)->disposing(
+ css::lang::EventObject(
+ static_cast< cppu::OWeakObject * >(this)));
+ } catch (const css::uno::RuntimeException & e) {
+ SAL_WARN(
+ "binaryurp",
+ "caught runtime exception '" << e.Message << '\'');
+ }
}
- tp = threadPool_;
- std::swap(reader_, r);
- std::swap(writer_, w);
- ls.swap(listeners_);
- terminated_ = true;
}
- try {
- connection_->close();
- } catch (const css::io::IOException & e) {
- SAL_INFO("binaryurp", "caught IO exception '" << e.Message << '\'');
- }
- assert(w.is());
- w->stop();
- joinThread(r.get());
- joinThread(w.get());
- assert(tp != 0);
- uno_threadpool_dispose(tp);
- Stubs s;
+ if (final) {
+ uno_threadpool_destroy(tp);
+ }
{
osl::MutexGuard g(mutex_);
- s.swap(stubs_);
- }
- for (Stubs::iterator i(s.begin()); i != s.end(); ++i) {
- for (Stub::iterator j(i->second.begin()); j != i->second.end(); ++j) {
- SAL_INFO(
- "binaryurp",
- "stub '" << i->first << "', '" << toString(j->first)
- << "' still mapped at Bridge::terminate");
- binaryUno_.get()->pExtEnv->revokeInterface(
- binaryUno_.get()->pExtEnv, j->second.object.get());
+ if (final) {
+ threadPool_ = 0;
}
}
- factory_->removeBridge(this);
- for (Listeners::iterator i(ls.begin()); i != ls.end(); ++i) {
- try {
- (*i)->disposing(
- css::lang::EventObject(
- static_cast< cppu::OWeakObject * >(this)));
- } catch (const css::uno::RuntimeException & e) {
- SAL_WARN(
- "binaryurp", "caught runtime exception '" << e.Message << '\'');
- }
- }
- uno_threadpool_destroy(tp);
+ terminated_.set();
}
@@ -341,2 +399,3 @@ uno_ThreadPool Bridge::getThreadPool() {
osl::MutexGuard g(mutex_);
+ checkDisposed();
assert(threadPool_ != 0);
@@ -347,9 +406,3 @@ rtl::Reference< Writer > Bridge::getWriter() {
osl::MutexGuard g(mutex_);
- if (terminated_) {
- throw css::lang::DisposedException(
- rtl::OUString(
- RTL_CONSTASCII_USTRINGPARAM(
- "Binary URP bridge already disposed")),
- static_cast< cppu::OWeakObject * >(this));
- }
+ checkDisposed();
assert(writer_.is());
@@ -824,5 +877,11 @@ bool Bridge::isCurrentContextMode() {
Bridge::~Bridge() {
- if (getThreadPool() != 0) {
- terminate();
+#if OSL_DEBUG_LEVEL > 0
+ {
+ osl::MutexGuard g(mutex_);
+ SAL_WARN_IF(
+ state_ == STATE_STARTED || state_ == STATE_TERMINATED, "binaryurp",
+ "undisposed bridge, potential deadlock ahead");
}
+#endif
+ dispose();
}
@@ -887,3 +946,7 @@ rtl::OUString Bridge::getDescription() throw (css::uno::RuntimeException) {
void Bridge::dispose() throw (css::uno::RuntimeException) {
- terminate();
+ // For terminate(true) not to deadlock, an external protocol must ensure
+ // that dispose is not called from a thread pool worker thread (that dispose
+ // is never called from the reader or writer thread is already ensured
+ // internally):
+ terminate(true);
// OOo expects dispose to not return while there are still remote calls in
@@ -902,3 +965,4 @@ void Bridge::addEventListener(
osl::MutexGuard g(mutex_);
- if (!terminated_) {
+ assert(state_ != STATE_INITIAL);
+ if (state_ == STATE_STARTED) {
listeners_.push_back(xListener);
@@ -997,3 +1061,14 @@ void Bridge::terminateWhenUnused(bool unused) {
// terminate cannot lead to deadlock):
- terminate();
+ terminate(false);
+ }
+}
+
+void Bridge::checkDisposed() {
+ assert(state_ != STATE_INITIAL);
+ if (state_ != STATE_STARTED) {
+ throw css::lang::DisposedException(
+ rtl::OUString(
+ RTL_CONSTASCII_USTRINGPARAM(
+ "Binary URP bridge already disposed")),
+ static_cast< cppu::OWeakObject * >(this));
}
diff --git a/binaryurp/source/bridge.hxx b/binaryurp/source/bridge.hxx
index 8d667897d253..3ffbfbaeb43b 100644
--- a/binaryurp/source/bridge.hxx
+++ b/binaryurp/source/bridge.hxx
@@ -95,4 +95,7 @@ public:
// Internally waits for all incoming and outgoing remote calls to terminate,
- // so must not be called from within such a call:
- void terminate();
+ // so must not be called from within such a call; when final is true, also
+ // joins all remaining threads (reader, writer, and worker threads from the
+ // thread pool), so must not be called with final set to true from such a
+ // thread:
+ void terminate(bool final);
@@ -230,2 +233,5 @@ private:
+ // Must only be called with mutex_ locked:
+ void checkDisposed();
+
typedef
@@ -242,2 +248,4 @@ private:
+ enum State { STATE_INITIAL, STATE_STARTED, STATE_TERMINATED, STATE_FINAL };
+
enum Mode {
@@ -261,4 +269,11 @@ private:
OutgoingRequests outgoingRequests_;
+ osl::Condition passive_;
+ // to guarantee that passive_ is eventually set (to avoid deadlock, see
+ // dispose), activeCalls_ only counts those calls for which it can be
+ // guaranteed that incrementActiveCalls is indeed followed by
+ // decrementActiveCalls, without an intervening exception
+ osl::Condition terminated_;
osl::Mutex mutex_;
+ State state_;
Listeners listeners_;
@@ -273,8 +288,2 @@ private:
std::size_t activeCalls_;
- osl::Condition passive_;
- // to guarantee that passive_ is eventually set (to avoid deadlock, see
- // dispose), activeCalls_ only counts those calls for which it can be
- // guaranteed that incrementActiveCalls is indeed followed by
- // decrementActiveCalls, without an intervening exception
- bool terminated_;
diff --git a/binaryurp/source/bridgefactory.cxx b/binaryurp/source/bridgefactory.cxx
index 5c9105585916..8f4caa64e17d 100644
--- a/binaryurp/source/bridgefactory.cxx
+++ b/binaryurp/source/bridgefactory.cxx
@@ -214,3 +214,3 @@ static cppu::ImplementationEntry const services[] = {
&binaryurp::BridgeFactory::static_getSupportedServiceNames,
- &cppu::createSingleComponentFactory, 0, 0 },
+ &cppu::createOneInstanceComponentFactory, 0, 0 },
{ 0, 0, 0, 0, 0, 0 }
diff --git a/binaryurp/source/incomingrequest.cxx b/binaryurp/source/incomingrequest.cxx
index 431c88505ad1..83b0030623e7 100644
--- a/binaryurp/source/incomingrequest.cxx
+++ b/binaryurp/source/incomingrequest.cxx
@@ -125,3 +125,3 @@ void IncomingRequest::execute() const {
}
- bridge_->terminate();
+ bridge_->terminate(false);
} else {
diff --git a/binaryurp/source/reader.cxx b/binaryurp/source/reader.cxx
index c052fadc5629..47bdfbf065fc 100644
--- a/binaryurp/source/reader.cxx
+++ b/binaryurp/source/reader.cxx
@@ -152,3 +152,3 @@ void Reader::run() {
}
- bridge_->terminate();
+ bridge_->terminate(false);
}
diff --git a/binaryurp/source/writer.cxx b/binaryurp/source/writer.cxx
index 5aca57b108eb..69825e13bc37 100644
--- a/binaryurp/source/writer.cxx
+++ b/binaryurp/source/writer.cxx
@@ -198,3 +198,3 @@ void Writer::run() {
}
- bridge_->terminate();
+ bridge_->terminate(false);
}
diff --git a/cppu/inc/uno/threadpool.h b/cppu/inc/uno/threadpool.h
index 0c647442e377..77bb712e53b4 100644
--- a/cppu/inc/uno/threadpool.h
+++ b/cppu/inc/uno/threadpool.h
@@ -168,6 +168,2 @@ uno_threadpool_putJob(
@param hPool The handle to be disposed.
- In case, hPool is 0, this function joins on all threads created
- by the threadpool administration. This may e.g. used to ensure, that
- no threads are inside the cppu library anymore, in case it needs to get
- unloaded.
@@ -182,2 +178,6 @@ uno_threadpool_dispose( uno_ThreadPool hPool ) SAL_THROW_EXTERN_C();
uno_threadpool_destroy().
+
+ A call to uno_threadpool_destroy can synchronously join on spawned worker
+ threads, so this function must never be called from such a worker thread.
+
@see uno_threadpool_create()
diff --git a/cppu/source/threadpool/thread.cxx b/cppu/source/threadpool/thread.cxx
index cc22a453c79d..12ea09a131ee 100644
--- a/cppu/source/threadpool/thread.cxx
+++ b/cppu/source/threadpool/thread.cxx
@@ -35,3 +35,2 @@
#include <com/sun/star/uno/XInterface.hpp>
-#include <rtl/instance.hxx>
#include <rtl/ustring.h>
@@ -50,13 +49,2 @@ namespace css = com::sun::star;
using namespace osl;
-extern "C" {
-
-void SAL_CALL cppu_requestThreadWorker( void *pVoid )
-{
- ::cppu_threadpool::ORequestThread *pThread = ( ::cppu_threadpool::ORequestThread * ) pVoid;
-
- pThread->run();
- pThread->onTerminated();
-}
-
-}
@@ -77,3 +65,3 @@ namespace cppu_threadpool {
- void ThreadAdmin::add( ORequestThread *p )
+ void ThreadAdmin::add( rtl::Reference< ORequestThread > const & p )
{
@@ -92,8 +80,15 @@ namespace cppu_threadpool {
- void ThreadAdmin::remove( ORequestThread * p )
+ void ThreadAdmin::remove_locked( rtl::Reference< ORequestThread > const & p )
+ {
+ ::std::list< rtl::Reference< ORequestThread > >::iterator ii = ::std::find( m_lst.begin(), m_lst.end(), p );
+ if( ii != m_lst.end() )
+ {
+ m_lst.erase( ii );
+ }
+ }
+
+ void ThreadAdmin::remove( rtl::Reference< ORequestThread > const & p )
{
MutexGuard aGuard( m_mutex );
- ::std::list< ORequestThread * >::iterator ii = ::std::find( m_lst.begin(), m_lst.end(), p );
- OSL_ASSERT( ii != m_lst.end() );
- m_lst.erase( ii );
+ remove_locked( p );
}
@@ -106,33 +101,16 @@ namespace cppu_threadpool {
}
- ORequestThread *pCurrent;
- do
+ for (;;)
{
- pCurrent = 0;
+ rtl::Reference< ORequestThread > pCurrent;
{
MutexGuard aGuard( m_mutex );
- if( ! m_lst.empty() )
+ if( m_lst.empty() )
{
- pCurrent = m_lst.front();
- pCurrent->setDeleteSelf( sal_False );
+ break;
}
+ pCurrent = m_lst.front();
+ m_lst.pop_front();
}
- if ( pCurrent )
- {
- pCurrent->join();
- delete pCurrent;
- }
- } while( pCurrent );
- }
-
- struct theThreadAdmin : public rtl::StaticWithInit< ThreadAdminHolder, theThreadAdmin >
- {
- ThreadAdminHolder operator () () {
- ThreadAdminHolder aRet(new ThreadAdmin());
- return aRet;
+ pCurrent->join();
}
- };
-
- ThreadAdminHolder& ThreadAdmin::getInstance()
- {
- return theThreadAdmin::get();
}
@@ -140,7 +118,7 @@ namespace cppu_threadpool {
// ----------------------------------------------------------------------------------
- ORequestThread::ORequestThread( JobQueue *pQueue,
+ ORequestThread::ORequestThread( ThreadPoolHolder const &aThreadPool,
+ JobQueue *pQueue,
const ByteSequence &aThreadId,
sal_Bool bAsynchron )
- : m_thread( 0 )
- , m_aThreadAdmin( ThreadAdmin::getInstance() )
+ : m_aThreadPool( aThreadPool )
, m_pQueue( pQueue )
@@ -148,16 +126,5 @@ namespace cppu_threadpool {
, m_bAsynchron( bAsynchron )
- , m_bDeleteSelf( sal_True )
- {
- m_aThreadAdmin->add( this );
- }
-
-
- ORequestThread::~ORequestThread()
- {
- if (m_thread != 0)
- {
- osl_destroyThread(m_thread);
- }
- }
+ {}
+ ORequestThread::~ORequestThread() {}
@@ -172,18 +139,20 @@ namespace cppu_threadpool {
- sal_Bool ORequestThread::create()
+ void ORequestThread::launch()
{
- OSL_ASSERT(m_thread == 0); // only one running thread per instance
-
- m_thread = osl_createSuspendedThread( cppu_requestThreadWorker, (void*)this);
- if ( m_thread )
- {
- osl_resumeThread( m_thread );
+ // Assumption is that osl::Thread::create returns normally with a true
+ // return value iff it causes osl::Thread::run to start executing:
+ acquire();
+ ThreadAdmin & rThreadAdmin = m_aThreadPool->getThreadAdmin();
+ osl::ClearableMutexGuard g(rThreadAdmin.m_mutex);
+ rThreadAdmin.add( this );
+ try {
+ if (!create()) {
+ throw std::runtime_error("osl::Thread::create failed");
+ }
+ } catch (...) {
+ rThreadAdmin.remove_locked( this );
+ g.clear();
+ release();
+ throw;
}
-
- return m_thread != 0;
- }
-
- void ORequestThread::join()
- {
- osl_joinWithThread( m_thread );
}
@@ -192,7 +161,4 @@ namespace cppu_threadpool {
{
- m_aThreadAdmin->remove( this );
- if( m_bDeleteSelf )
- {
- delete this;
- }
+ m_aThreadPool->getThreadAdmin().remove( this );
+ release();
}
@@ -201,41 +167,49 @@ namespace cppu_threadpool {
{
- ThreadPoolHolder theThreadPool = cppu_threadpool::ThreadPool::getInstance();
-
- while ( m_pQueue )
+ try
{
- if( ! m_bAsynchron )
+ while ( m_pQueue )
{
- if ( !uno_bindIdToCurrentThread( m_aThreadId.getHandle() ) )
+ if( ! m_bAsynchron )
{
- OSL_ASSERT( false );
+ if ( !uno_bindIdToCurrentThread( m_aThreadId.getHandle() ) )
+ {
+ OSL_ASSERT( false );
+ }
}
- }
- while( ! m_pQueue->isEmpty() )
- {
- // Note : Oneways should not get a disposable disposeid,
- // It does not make sense to dispose a call in this state.
- // That's way we put it an disposeid, that can't be used otherwise.
- m_pQueue->enter(
- sal::static_int_cast< sal_Int64 >(
- reinterpret_cast< sal_IntPtr >(this)),
- sal_True );
-
- if( m_pQueue->isEmpty() )
+ while( ! m_pQueue->isEmpty() )
{
- theThreadPool->revokeQueue( m_aThreadId , m_bAsynchron );
- // Note : revokeQueue might have failed because m_pQueue.isEmpty()
- // may be false (race).
+ // Note : Oneways should not get a disposable disposeid,
+ // It does not make sense to dispose a call in this state.
+ // That's way we put it an disposeid, that can't be used otherwise.
+ m_pQueue->enter(
+ sal::static_int_cast< sal_Int64 >(
+ reinterpret_cast< sal_IntPtr >(this)),
+ sal_True );
+
+ if( m_pQueue->isEmpty() )
+ {
+ m_aThreadPool->revokeQueue( m_aThreadId , m_bAsynchron );
+ // Note : revokeQueue might have failed because m_pQueue.isEmpty()
+ // may be false (race).
+ }
}
- }
- delete m_pQueue;
- m_pQueue = 0;
+ delete m_pQueue;
+ m_pQueue = 0;
- if( ! m_bAsynchron )
- {
- uno_releaseIdFromCurrentThread();
- }
+ if( ! m_bAsynchron )
+ {
+ uno_releaseIdFromCurrentThread();
+ }
- theThreadPool->waitInPool( this );
+ m_aThreadPool->waitInPool( this );
+ }
+ }
+ catch (...)
+ {
+ // Work around the problem that onTerminated is not called if run
+ // throws an exception:
+ onTerminated();
+ throw;
}
diff --git a/cppu/source/threadpool/thread.hxx b/cppu/source/threadpool/thread.hxx
index a3ea45aadaed..88f3d91f8722 100644
--- a/cppu/source/threadpool/thread.hxx
+++ b/cppu/source/threadpool/thread.hxx
@@ -30,8 +30,8 @@
-#include <list>
+#include <osl/thread.hxx>
#include <sal/types.h>
-
-#include <osl/thread.h>
+#include <salhelper/simplereferenceobject.hxx>
#include "jobqueue.hxx"
+#include "threadpool.hxx"
@@ -40,4 +40,2 @@ namespace cppu_threadpool {
class JobQueue;
- class ThreadAdmin;
- typedef boost::shared_ptr<ThreadAdmin> ThreadAdminHolder;
@@ -47,9 +45,11 @@ namespace cppu_threadpool {
//-----------------------------------------
- class ORequestThread
+ class ORequestThread:
+ public salhelper::SimpleReferenceObject, public osl::Thread
{
public:
- ORequestThread( JobQueue * ,
+ ORequestThread( ThreadPoolHolder const &aThreadPool,
+ JobQueue * ,
const ::rtl::ByteSequence &aThreadId,
sal_Bool bAsynchron );
- ~ORequestThread();
+ virtual ~ORequestThread();
@@ -57,12 +57,15 @@ namespace cppu_threadpool {
- sal_Bool create();
- void join();
- void onTerminated();
- void run();
- inline void setDeleteSelf( sal_Bool b )
- { m_bDeleteSelf = b; }
+ void launch();
+
+ static inline void * operator new(std::size_t size)
+ { return SimpleReferenceObject::operator new(size); }
+
+ static inline void operator delete(void * pointer)
+ { SimpleReferenceObject::operator delete(pointer); }
private:
- oslThread m_thread;
- ThreadAdminHolder m_aThreadAdmin;
+ virtual void SAL_CALL run();
+ virtual void SAL_CALL onTerminated();
+
+ ThreadPoolHolder m_aThreadPool;
JobQueue *m_pQueue;
@@ -70,19 +73,2 @@ namespace cppu_threadpool {
sal_Bool m_bAsynchron;
- sal_Bool m_bDeleteSelf;
- };
-
- class ThreadAdmin
- {
- public:
- ThreadAdmin();
- ~ThreadAdmin ();
- static ThreadAdminHolder &getInstance();
- void add( ORequestThread * );
- void remove( ORequestThread * );
- void join();
-
- private:
- ::osl::Mutex m_mutex;
- ::std::list< ORequestThread * > m_lst;
- bool m_disposed;
};
diff --git a/cppu/source/threadpool/threadpool.cxx b/cppu/source/threadpool/threadpool.cxx
index f9f0be6c7d03..9099b7e1981e 100644
--- a/cppu/source/threadpool/threadpool.cxx
+++ b/cppu/source/threadpool/threadpool.cxx
@@ -111,11 +111,2 @@ namespace cppu_threadpool
- struct theThreadPool :
- public rtl::StaticWithInit< ThreadPoolHolder, theThreadPool >
- {
- ThreadPoolHolder operator () () {
- ThreadPoolHolder aRet(new ThreadPool());
- return aRet;
- }
- };
-
ThreadPool::ThreadPool()
@@ -134,7 +125,2 @@ namespace cppu_threadpool
}
- ThreadPoolHolder ThreadPool::getInstance()
- {
- return theThreadPool::get();
- }
-
@@ -142,34 +128,17 @@ namespace cppu_threadpool
{
- if( nDisposeId )
- {
- m_DisposedCallerAdmin->dispose( nDisposeId );
+ m_DisposedCallerAdmin->dispose( nDisposeId );
- MutexGuard guard( m_mutex );
- for( ThreadIdHashMap::iterator ii = m_mapQueue.begin() ;
- ii != m_mapQueue.end();
- ++ii)
+ MutexGuard guard( m_mutex );
+ for( ThreadIdHashMap::iterator ii = m_mapQueue.begin() ;
+ ii != m_mapQueue.end();
+ ++ii)
+ {
+ if( (*ii).second.first )
{
- if( (*ii).second.first )
- {
- (*ii).second.first->dispose( nDisposeId );
- }
- if( (*ii).second.second )
- {
- (*ii).second.second->dispose( nDisposeId );
- }
+ (*ii).second.first->dispose( nDisposeId );
}
- }
- else
- {
+ if( (*ii).second.second )
{
- MutexGuard guard( m_mutexWaitingThreadList );
- for( WaitingThreadList::iterator ii = m_lstThreads.begin() ;
- ii != m_lstThreads.end() ;
- ++ ii )
- {
- // wake the threads up
- osl_setCondition( (*ii)->condition );
- }
+ (*ii).second.second->dispose( nDisposeId );
}
- ThreadAdmin::getInstance()->join();
}
@@ -187,3 +156,3 @@ namespace cppu_threadpool
******************/
- void ThreadPool::waitInPool( ORequestThread * pThread )
+ void ThreadPool::waitInPool( rtl::Reference< ORequestThread > const & pThread )
{
@@ -203,3 +172,3 @@ namespace cppu_threadpool
MutexGuard guard ( m_mutexWaitingThreadList );
- if( waitingThread.thread )
+ if( waitingThread.thread.is() )
{
@@ -216,2 +185,17 @@ namespace cppu_threadpool
+ void ThreadPool::joinWorkers()
+ {
+ {
+ MutexGuard guard( m_mutexWaitingThreadList );
+ for( WaitingThreadList::iterator ii = m_lstThreads.begin() ;
+ ii != m_lstThreads.end() ;
+ ++ ii )
+ {
+ // wake the threads up
+ osl_setCondition( (*ii)->condition );
+ }
+ }
+ m_aThreadAdmin.join();
+ }
+
void ThreadPool::createThread( JobQueue *pQueue ,
@@ -242,6 +226,5 @@ namespace cppu_threadpool
{
- ORequestThread *pThread =
- new ORequestThread( pQueue , aThreadId, bAsynchron);
- // deletes itself !
- pThread->create();
+ rtl::Reference< ORequestThread > pThread(
+ new ORequestThread( this, pQueue , aThreadId, bAsynchron) );
+ pThread->launch();
}
@@ -387,2 +370,8 @@ namespace cppu_threadpool
+// All uno_ThreadPool handles in g_pThreadpoolHashSet with overlapping life
+// spans share one ThreadPool instance. When g_pThreadpoolHashSet becomes empty
+// (within the last uno_threadpool_destroy) all worker threads spawned by that
+// ThreadPool instance are joined (which implies that uno_threadpool_destroy
+// must never be called from a worker thread); afterwards, the next call to
+// uno_threadpool_create (if any) will lead to a new ThreadPool instance.
@@ -417,2 +406,15 @@ struct _uno_ThreadPool
+namespace {
+
+ThreadPoolHolder getThreadPool( uno_ThreadPool hPool )
+{
+ MutexGuard guard( Mutex::getGlobalMutex() );
+ assert( g_pThreadpoolHashSet != 0 );
+ ThreadpoolHashSet::iterator i( g_pThreadpoolHashSet->find(hPool) );
+ assert( i != g_pThreadpoolHashSet->end() );
+ return i->second;
+}
+
+}
+
extern "C" uno_ThreadPool SAL_CALL
@@ -421,2 +423,3 @@ uno_threadpool_create() SAL_THROW_EXTERN_C()
MutexGuard guard( Mutex::getGlobalMutex() );
+ ThreadPoolHolder p;
if( ! g_pThreadpoolHashSet )
@@ -424,2 +427,8 @@ uno_threadpool_create() SAL_THROW_EXTERN_C()
g_pThreadpoolHashSet = new ThreadpoolHashSet();
+ p = new ThreadPool;
+ }
+ else
+ {
+ assert( !g_pThreadpoolHashSet->empty() );
+ p = g_pThreadpoolHashSet->begin()->second;
}
@@ -428,3 +437,3 @@ uno_threadpool_create() SAL_THROW_EXTERN_C()
uno_ThreadPool h = new struct _uno_ThreadPool;
- g_pThreadpoolHashSet->insert( ThreadpoolHashSet::value_type(h, ThreadPool::getInstance()) );
+ g_pThreadpoolHashSet->insert( ThreadpoolHashSet::value_type(h, p) );
return h;
@@ -433,3 +442,3 @@ uno_threadpool_create() SAL_THROW_EXTERN_C()
extern "C" void SAL_CALL
-uno_threadpool_attach( uno_ThreadPool ) SAL_THROW_EXTERN_C()
+uno_threadpool_attach( uno_ThreadPool hPool ) SAL_THROW_EXTERN_C()
{
@@ -437,3 +446,3 @@ uno_threadpool_attach( uno_ThreadPool ) SAL_THROW_EXTERN_C()
uno_getIdOfCurrentThread( &pThreadId );
- ThreadPool::getInstance()->prepare( pThreadId );
+ getThreadPool( hPool )->prepare( pThreadId );
rtl_byte_sequence_release( pThreadId );
@@ -449,3 +458,3 @@ uno_threadpool_enter( uno_ThreadPool hPool , void **ppJob )
*ppJob =
- ThreadPool::getInstance()->enter(
+ getThreadPool( hPool )->enter(
pThreadId,
@@ -465,3 +474,3 @@ extern "C" void SAL_CALL
uno_threadpool_putJob(
- uno_ThreadPool,
+ uno_ThreadPool hPool,
sal_Sequence *pThreadId,
@@ -471,3 +480,3 @@ uno_threadpool_putJob(
{
- ThreadPool::getInstance()->addJob( pThreadId, bIsOneway, pJob ,doRequest );
+ getThreadPool(hPool)->addJob( pThreadId, bIsOneway, pJob ,doRequest );
}
@@ -477,3 +486,3 @@ uno_threadpool_dispose( uno_ThreadPool hPool ) SAL_THROW_EXTERN_C()
{
- ThreadPool::getInstance()->dispose(
+ getThreadPool(hPool)->dispose(
sal::static_int_cast< sal_Int64 >(
@@ -485,5 +494,4 @@ uno_threadpool_destroy( uno_ThreadPool hPool ) SAL_THROW_EXTERN_C()
{
- assert(hPool != 0);
-
- ThreadPool::getInstance()->destroy(
+ ThreadPoolHolder p( getThreadPool(hPool) );
+ p->destroy(
sal::static_int_cast< sal_Int64 >(
@@ -512,3 +520,3 @@ uno_threadpool_destroy( uno_ThreadPool hPool ) SAL_THROW_EXTERN_C()
{
- uno_threadpool_dispose( 0 );
+ p->joinWorkers();
}
diff --git a/cppu/source/threadpool/threadpool.hxx b/cppu/source/threadpool/threadpool.hxx
index 18bb47a1ff20..3ff52b362af1 100644
--- a/cppu/source/threadpool/threadpool.hxx
+++ b/cppu/source/threadpool/threadpool.hxx
@@ -27,2 +27,8 @@
************************************************************************/
+
+#ifndef INCLUDED_CPPU_SOURCE_THREADPOOL_THREADPOOL_HXX
+#define INCLUDED_CPPU_SOURCE_THREADPOOL_THREADPOOL_HXX
+
+#include <list>
+
#include <boost/unordered_map.hpp>
@@ -32,2 +38,4 @@
#include <rtl/byteseq.hxx>
+#include <rtl/ref.hxx>
+#include <salhelper/simplereferenceobject.hxx>
@@ -76,3 +84,3 @@ namespace cppu_threadpool {
oslCondition condition;
- ORequestThread *thread;
+ rtl::Reference< ORequestThread > thread;
};
@@ -100,6 +108,24 @@ namespace cppu_threadpool {
+ class ThreadAdmin
+ {
+ public:
+ ThreadAdmin();
+ ~ThreadAdmin ();
+
+ void add( rtl::Reference< ORequestThread > const & );
+ void remove( rtl::Reference< ORequestThread > const & );
+ void join();
+
+ void remove_locked( rtl::Reference< ORequestThread > const & );
+ ::osl::Mutex m_mutex;
+
+ private:
+ ::std::list< rtl::Reference< ORequestThread > > m_lst;
+ bool m_disposed;
+ };
+
class ThreadPool;
- typedef boost::shared_ptr<ThreadPool> ThreadPoolHolder;
+ typedef rtl::Reference<ThreadPool> ThreadPoolHolder;
- class ThreadPool
+ class ThreadPool: public salhelper::SimpleReferenceObject
{
@@ -108,3 +134,2 @@ namespace cppu_threadpool {
~ThreadPool();
- static ThreadPoolHolder getInstance();
@@ -126,3 +151,8 @@ namespace cppu_threadpool {
- void waitInPool( ORequestThread *pThread );
+ void waitInPool( rtl::Reference< ORequestThread > const & pThread );
+
+ void joinWorkers();
+
+ ThreadAdmin & getThreadAdmin() { return m_aThreadAdmin; }
+
private:
@@ -138,2 +168,3 @@ namespace cppu_threadpool {
DisposedCallerAdminHolder m_DisposedCallerAdmin;
+ ThreadAdmin m_aThreadAdmin;
};
@@ -142,2 +173,4 @@ namespace cppu_threadpool {
+#endif
+
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/cppu/util/makefile.mk b/cppu/util/makefile.mk
index ea0c09488c85..cf8e1bd5133a 100644
--- a/cppu/util/makefile.mk
+++ b/cppu/util/makefile.mk
@@ -58,3 +58,3 @@ SHL1TARGET= uno_$(TARGET)
-SHL1STDLIBS = $(SALLIB)
+SHL1STDLIBS = $(SALLIB) $(SALHELPERLIB)
diff --git a/cpputools/source/unoexe/unoexe.cxx b/cpputools/source/unoexe/unoexe.cxx
index 684bc39934e1..46fcbf8b310a 100644
--- a/cpputools/source/unoexe/unoexe.cxx
+++ b/cpputools/source/unoexe/unoexe.cxx
@@ -849,2 +849,6 @@ SAL_IMPLEMENT_MAIN_WITH_ARGS(argc,)
ODisposingListener::waitFor( xComp );
+ xComp->dispose();
+ // explicitly dispose the remote bridge so that it joins
+ // on all spawned threads before process exit (see
+ // binaryurp/source/bridge.cxx for details)
break;