[PATCH] Fixed ThreadPool (and dependent ORequestThread) life cycle
Stephan Bergmann
sbergman at redhat.com
Wed May 16 13:09:21 PDT 2012
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.
(cherry picked from commit d015384e1d98fe77fd59339044f58efb1ab9fb25)
Conflicts:
binaryurp/source/bridge.cxx
Change-Id: I45fa76e80e790a11065e7bf8ac9d92af2e62f262
---
binaryurp/source/bridge.cxx | 46 ++++++++++++++++++++++----------
binaryurp/source/bridge.hxx | 4 +-
cppu/source/threadpool/threadpool.cxx | 24 ++++++++++++-----
cppu/source/threadpool/threadpool.hxx | 4 +-
4 files changed, 52 insertions(+), 26 deletions(-)
diff --git a/binaryurp/source/bridge.cxx b/binaryurp/source/bridge.cxx
index b491a2a..f591fe0 100644
--- a/binaryurp/source/bridge.cxx
+++ b/binaryurp/source/bridge.cxx
@@ -234,16 +234,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_->create();
- reader_.set(new Reader(this));
- reader_->create();
+ 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_->create() 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_->create() can lead to an early call to
+ // Bridge::terminate
+ w->create();
+ r->create();
}
void Bridge::terminate() {
+ uno_ThreadPool tp;
rtl::Reference< Reader > r;
rtl::Reference< Writer > w;
Listeners ls;
@@ -252,6 +264,7 @@ void Bridge::terminate() {
if (terminated_) {
return;
}
+ tp = threadPool_;
std::swap(reader_, r);
std::swap(writer_, w);
ls.swap(listeners_);
@@ -266,8 +279,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_);
@@ -294,6 +307,7 @@ void Bridge::terminate() {
"binaryurp", "caught runtime exception '" << e.Message << '\'');
}
}
+ uno_threadpool_destroy(tp);
}
css::uno::Reference< css::connection::XConnection > Bridge::getConnection()
@@ -323,7 +337,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_;
}
@@ -564,7 +579,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));
@@ -575,7 +591,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();
@@ -806,8 +822,8 @@ bool Bridge::isCurrentContextMode() {
}
Bridge::~Bridge() {
- if (threadPool_ != 0) {
- uno_threadpool_destroy(threadPool_);
+ if (getThreadPool() != 0) {
+ terminate();
}
}
@@ -934,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 cf281f2..8d66789 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_;
diff --git a/cppu/source/threadpool/threadpool.cxx b/cppu/source/threadpool/threadpool.cxx
index 312fd89..f9f0be6 100644
--- a/cppu/source/threadpool/threadpool.cxx
+++ b/cppu/source/threadpool/threadpool.cxx
@@ -26,7 +26,10 @@
*
************************************************************************/
+#include "sal/config.h"
+
#include <boost/unordered_map.hpp>
+#include <cassert>
#include <stdio.h>
#include <osl/diagnose.h>
@@ -73,7 +76,7 @@ namespace cppu_threadpool
m_lst.push_back( nDisposeId );
}
- void DisposedCallerAdmin::stopDisposing( sal_Int64 nDisposeId )
+ void DisposedCallerAdmin::destroy( sal_Int64 nDisposeId )
{
MutexGuard guard( m_mutex );
for( DisposedCallerList::iterator ii = m_lst.begin() ;
@@ -172,9 +175,9 @@ namespace cppu_threadpool
}
}
- void ThreadPool::stopDisposing( sal_Int64 nDisposeId )
+ void ThreadPool::destroy( sal_Int64 nDisposeId )
{
- m_DisposedCallerAdmin->stopDisposing( nDisposeId );
+ m_DisposedCallerAdmin->destroy( nDisposeId );
}
/******************
@@ -480,13 +483,14 @@ uno_threadpool_dispose( uno_ThreadPool hPool ) SAL_THROW_EXTERN_C()
extern "C" void SAL_CALL
uno_threadpool_destroy( uno_ThreadPool hPool ) SAL_THROW_EXTERN_C()
{
- ThreadPool::getInstance()->stopDisposing(
+ assert(hPool != 0);
+
+ ThreadPool::getInstance()->destroy(
sal::static_int_cast< sal_Int64 >(
reinterpret_cast< sal_IntPtr >(hPool)) );
- if( hPool )
+ bool empty;
{
- // special treatment for 0 !
OSL_ASSERT( g_pThreadpoolHashSet );
MutexGuard guard( Mutex::getGlobalMutex() );
@@ -496,12 +500,18 @@ uno_threadpool_destroy( uno_ThreadPool hPool ) SAL_THROW_EXTERN_C()
g_pThreadpoolHashSet->erase( ii );
delete hPool;
- if( g_pThreadpoolHashSet->empty() )
+ empty = g_pThreadpoolHashSet->empty();
+ if( empty )
{
delete g_pThreadpoolHashSet;
g_pThreadpoolHashSet = 0;
}
}
+
+ if( empty )
+ {
+ uno_threadpool_dispose( 0 );
+ }
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/cppu/source/threadpool/threadpool.hxx b/cppu/source/threadpool/threadpool.hxx
index 273798c..18bb47a 100644
--- a/cppu/source/threadpool/threadpool.hxx
+++ b/cppu/source/threadpool/threadpool.hxx
@@ -90,7 +90,7 @@ namespace cppu_threadpool {
static DisposedCallerAdminHolder getInstance();
void dispose( sal_Int64 nDisposeId );
- void stopDisposing( sal_Int64 nDisposeId );
+ void destroy( sal_Int64 nDisposeId );
sal_Bool isDisposed( sal_Int64 nDisposeId );
private:
@@ -109,7 +109,7 @@ namespace cppu_threadpool {
static ThreadPoolHolder getInstance();
void dispose( sal_Int64 nDisposeId );
- void stopDisposing( sal_Int64 nDisposeId );
+ void destroy( sal_Int64 nDisposeId );
void addJob( const ByteSequence &aThreadId,
sal_Bool bAsynchron,
--
1.7.7.6
--------------040804040903010005080009--
More information about the LibreOffice
mailing list