From d015384e1d98fe77fd59339044f58efb1ab9fb25 Mon Sep 17 00:00:00 2001 From: Stephan Bergmann Date: Wed, 16 May 2012 22:09:21 +0200 Subject: Fixed ThreadPool (and dependent ORequestThread) life cycle At least with sw_complex test under load, it happened that an ORequestThread could still process a remote release request while the main thread was already in exit(3). This was because (a) ThreadPool never joined with the spawned worker threads (which has been rectified by calling uno_threadpool_dispose(0) from the final uno_threadpool_destroy), and (b) binaryurp::Bridge called uno_threadpool_destroy only from its destructor (which could go as late as exit(3)) instead of from terminate. Additional clean up: * Access to Bridge's threadPool_ is now cleanly controlled by mutex_ (even though that might not be necessary in every case). * ThreadPool's stopDisposing got renamed to destroy, to make meaning clearer. Change-Id: I45fa76e80e790a11065e7bf8ac9d92af2e62f262 --- binaryurp/source/bridge.cxx | 52 +++++++++++++++++++++++++++------------------ binaryurp/source/bridge.hxx | 4 ++-- 2 files changed, 33 insertions(+), 23 deletions(-) (limited to 'binaryurp') diff --git a/binaryurp/source/bridge.cxx b/binaryurp/source/bridge.cxx index 50b873f6a70f..2d1f622369a3 100644 --- a/binaryurp/source/bridge.cxx +++ b/binaryurp/source/bridge.cxx @@ -235,22 +235,28 @@ Bridge::Bridge( } void Bridge::start() { - assert(threadPool_ == 0 && !writer_.is() && !reader_.is()); - threadPool_ = uno_threadpool_create(); - assert(threadPool_ != 0); - writer_.set(new Writer(this)); - writer_->launch(); - reader_.set(new Reader(this)); - reader_->launch(); - // it is important to call reader_->launch() last here; both - // Writer::execute and Reader::execute can call Bridge::terminate, but - // Writer::execute is initially blocked in unblocked_.wait() until - // Reader::execute has called bridge_->sendRequestChangeRequest(), so - // effectively only reader_->launch() can lead to an early call to - // Bridge::terminate + rtl::Reference< Reader > r(new Reader(this)); + rtl::Reference< Writer > w(new Writer(this)); + { + osl::MutexGuard g(mutex_); + assert(threadPool_ == 0 && !writer_.is() && !reader_.is()); + threadPool_ = uno_threadpool_create(); + assert(threadPool_ != 0); + reader_ = r; + writer_ = w; + } + // It is important to call reader_->launch() last here; both + // Writer::execute and Reader::execute can call Bridge::terminate, but + // Writer::execute is initially blocked in unblocked_.wait() until + // Reader::execute has called bridge_->sendRequestChangeRequest(), so + // effectively only reader_->launch() can lead to an early call to + // Bridge::terminate + w->launch(); + r->launch(); } void Bridge::terminate() { + uno_ThreadPool tp; rtl::Reference< Reader > r; rtl::Reference< Writer > w; Listeners ls; @@ -259,6 +265,7 @@ void Bridge::terminate() { if (terminated_) { return; } + tp = threadPool_; std::swap(reader_, r); std::swap(writer_, w); ls.swap(listeners_); @@ -273,8 +280,8 @@ void Bridge::terminate() { w->stop(); joinThread(r.get()); joinThread(w.get()); - assert(threadPool_ != 0); - uno_threadpool_dispose(threadPool_); + assert(tp != 0); + uno_threadpool_dispose(tp); Stubs s; { osl::MutexGuard g(mutex_); @@ -301,6 +308,7 @@ void Bridge::terminate() { "binaryurp", "caught runtime exception '" << e.Message << '\''); } } + uno_threadpool_destroy(tp); } css::uno::Reference< css::connection::XConnection > Bridge::getConnection() @@ -330,7 +338,8 @@ BinaryAny Bridge::mapCppToBinaryAny(css::uno::Any const & cppAny) { return out; } -uno_ThreadPool Bridge::getThreadPool() const { +uno_ThreadPool Bridge::getThreadPool() { + osl::MutexGuard g(mutex_); assert(threadPool_ != 0); return threadPool_; } @@ -571,7 +580,8 @@ bool Bridge::makeCall( { std::auto_ptr< IncomingReply > resp; { - AttachThread att(threadPool_); + uno_ThreadPool tp = getThreadPool(); + AttachThread att(tp); PopOutgoingRequest pop( outgoingRequests_, att.getTid(), OutgoingRequest(OutgoingRequest::KIND_NORMAL, member, setter)); @@ -582,7 +592,7 @@ bool Bridge::makeCall( incrementCalls(true); incrementActiveCalls(); void * job; - uno_threadpool_enter(threadPool_, &job); + uno_threadpool_enter(tp, &job); resp.reset(static_cast< IncomingReply * >(job)); decrementActiveCalls(); decrementCalls(); @@ -812,8 +822,8 @@ bool Bridge::isCurrentContextMode() { } Bridge::~Bridge() { - if (threadPool_ != 0) { - uno_threadpool_destroy(threadPool_); + if (getThreadPool() != 0) { + terminate(); } } @@ -940,7 +950,7 @@ void Bridge::sendProtPropRequest( void Bridge::makeReleaseCall( rtl::OUString const & oid, css::uno::TypeDescription const & type) { - AttachThread att(threadPool_); + AttachThread att(getThreadPool()); sendRequest( att.getTid(), oid, type, css::uno::TypeDescription( diff --git a/binaryurp/source/bridge.hxx b/binaryurp/source/bridge.hxx index cf281f2febfd..8d667897d253 100644 --- a/binaryurp/source/bridge.hxx +++ b/binaryurp/source/bridge.hxx @@ -106,7 +106,7 @@ public: BinaryAny mapCppToBinaryAny(com::sun::star::uno::Any const & cppAny); - uno_ThreadPool getThreadPool() const; + uno_ThreadPool getThreadPool(); rtl::Reference< Writer > getWriter(); @@ -258,11 +258,11 @@ private: com::sun::star::uno::TypeDescription protPropType_; com::sun::star::uno::TypeDescription protPropRequest_; com::sun::star::uno::TypeDescription protPropCommit_; - uno_ThreadPool threadPool_; OutgoingRequests outgoingRequests_; osl::Mutex mutex_; Listeners listeners_; + uno_ThreadPool threadPool_; rtl::Reference< Writer > writer_; rtl::Reference< Reader > reader_; bool currentContextMode_; -- cgit