Kaydet (Commit) b021fdfa authored tarafından Stephan Bergmann's avatar Stephan Bergmann

cid#983623 Don't throw DisposedException past uno_threadpool_putJob

This improves on b68640c4 "Prevent creation of
new ORequestThreads during shutdown," which added throwing the DisposedException
from ThreadAdmin::add.  But ThreadAdmin::m_disposed can only become true via
uno_threadpool_destroy -> ThreadPool::joinWorkers -> ThreadAdmin::join, and
ThreadAdmin::add observing that can only happen via uno_threadpool_putJob ->
ThreadPool::addJob -> ThreadPool::createThread -> ORequestThread::launch ->
ThradAdmin::add, where the bridges should ensure that uno_threadpool_destroy
does not run in parallel with uno_threadpool_putJob.  So demote this from a
DisposedException to a SAL_WARN.

Change-Id: I3912ea077b7fa35827c41e82dd0a8f962ba412b6
üst 9dacd849
...@@ -21,12 +21,6 @@ ...@@ -21,12 +21,6 @@
#include <osl/diagnose.h> #include <osl/diagnose.h>
#include <uno/threadpool.h> #include <uno/threadpool.h>
#include <com/sun/star/lang/DisposedException.hpp>
#include <com/sun/star/uno/Reference.hxx>
#include <com/sun/star/uno/XInterface.hpp>
#include <rtl/ustring.h>
#include <rtl/ustring.hxx>
#include "thread.hxx" #include "thread.hxx"
#include "jobqueue.hxx" #include "jobqueue.hxx"
#include "threadpool.hxx" #include "threadpool.hxx"
...@@ -49,16 +43,15 @@ namespace cppu_threadpool { ...@@ -49,16 +43,15 @@ namespace cppu_threadpool {
#endif #endif
} }
void ThreadAdmin::add( rtl::Reference< ORequestThread > const & p ) bool ThreadAdmin::add( rtl::Reference< ORequestThread > const & p )
{ {
MutexGuard aGuard( m_mutex ); MutexGuard aGuard( m_mutex );
if( m_disposed ) if( m_disposed )
{ {
throw css::lang::DisposedException( return false;
"cppu_threadpool::ORequestThread created after"
" cppu_threadpool::ThreadAdmin has been disposed");
} }
m_lst.push_back( p ); m_lst.push_back( p );
return true;
} }
void ThreadAdmin::remove_locked( rtl::Reference< ORequestThread > const & p ) void ThreadAdmin::remove_locked( rtl::Reference< ORequestThread > const & p )
...@@ -120,14 +113,16 @@ namespace cppu_threadpool { ...@@ -120,14 +113,16 @@ namespace cppu_threadpool {
m_bAsynchron = bAsynchron; m_bAsynchron = bAsynchron;
} }
void ORequestThread::launch() bool ORequestThread::launch()
{ {
// Assumption is that osl::Thread::create returns normally with a true // Assumption is that osl::Thread::create returns normally with a true
// return value iff it causes osl::Thread::run to start executing: // return value iff it causes osl::Thread::run to start executing:
acquire(); acquire();
ThreadAdmin & rThreadAdmin = m_aThreadPool->getThreadAdmin(); ThreadAdmin & rThreadAdmin = m_aThreadPool->getThreadAdmin();
osl::ClearableMutexGuard g(rThreadAdmin.m_mutex); osl::ClearableMutexGuard g(rThreadAdmin.m_mutex);
rThreadAdmin.add( this ); if (!rThreadAdmin.add( this )) {
return false;
}
try { try {
if (!create()) { if (!create()) {
throw std::runtime_error("osl::Thread::create failed"); throw std::runtime_error("osl::Thread::create failed");
...@@ -138,6 +133,7 @@ namespace cppu_threadpool { ...@@ -138,6 +133,7 @@ namespace cppu_threadpool {
release(); release();
throw; throw;
} }
return true;
} }
void ORequestThread::onTerminated() void ORequestThread::onTerminated()
......
...@@ -45,7 +45,7 @@ namespace cppu_threadpool { ...@@ -45,7 +45,7 @@ namespace cppu_threadpool {
void setTask( JobQueue * , const ::rtl::ByteSequence & aThreadId , bool bAsynchron ); void setTask( JobQueue * , const ::rtl::ByteSequence & aThreadId , bool bAsynchron );
void launch(); bool launch();
static inline void * operator new(std::size_t size) static inline void * operator new(std::size_t size)
{ return SimpleReferenceObject::operator new(size); } { return SimpleReferenceObject::operator new(size); }
......
...@@ -27,6 +27,7 @@ ...@@ -27,6 +27,7 @@
#include <osl/mutex.hxx> #include <osl/mutex.hxx>
#include <osl/thread.h> #include <osl/thread.h>
#include <rtl/instance.hxx> #include <rtl/instance.hxx>
#include <sal/log.hxx>
#include <uno/threadpool.h> #include <uno/threadpool.h>
...@@ -190,11 +191,10 @@ namespace cppu_threadpool ...@@ -190,11 +191,10 @@ namespace cppu_threadpool
m_aThreadAdmin.join(); m_aThreadAdmin.join();
} }
void ThreadPool::createThread( JobQueue *pQueue , bool ThreadPool::createThread( JobQueue *pQueue ,
const ByteSequence &aThreadId, const ByteSequence &aThreadId,
bool bAsynchron ) bool bAsynchron )
{ {
bool bCreate = true;
{ {
// Can a thread be reused ? // Can a thread be reused ?
MutexGuard guard( m_mutexWaitingThreadList ); MutexGuard guard( m_mutexWaitingThreadList );
...@@ -210,16 +210,13 @@ namespace cppu_threadpool ...@@ -210,16 +210,13 @@ namespace cppu_threadpool
// let the thread go // let the thread go
osl_setCondition( pWaitingThread->condition ); osl_setCondition( pWaitingThread->condition );
bCreate = false; return true;
} }
} }
if( bCreate ) rtl::Reference< ORequestThread > pThread(
{ new ORequestThread( this, pQueue , aThreadId, bAsynchron) );
rtl::Reference< ORequestThread > pThread( return pThread->launch();
new ORequestThread( this, pQueue , aThreadId, bAsynchron) );
pThread->launch();
}
} }
bool ThreadPool::revokeQueue( const ByteSequence &aThreadId, bool bAsynchron ) bool ThreadPool::revokeQueue( const ByteSequence &aThreadId, bool bAsynchron )
...@@ -264,7 +261,7 @@ namespace cppu_threadpool ...@@ -264,7 +261,7 @@ namespace cppu_threadpool
} }
void ThreadPool::addJob( bool ThreadPool::addJob(
const ByteSequence &aThreadId , const ByteSequence &aThreadId ,
bool bAsynchron, bool bAsynchron,
void *pThreadSpecificData, void *pThreadSpecificData,
...@@ -310,10 +307,7 @@ namespace cppu_threadpool ...@@ -310,10 +307,7 @@ namespace cppu_threadpool
pQueue->add( pThreadSpecificData , doRequest ); pQueue->add( pThreadSpecificData , doRequest );
} }
if( bCreateThread ) return !bCreateThread || createThread( pQueue , aThreadId , bAsynchron);
{
createThread( pQueue , aThreadId , bAsynchron);
}
} }
void ThreadPool::prepare( const ByteSequence &aThreadId ) void ThreadPool::prepare( const ByteSequence &aThreadId )
...@@ -470,7 +464,12 @@ uno_threadpool_putJob( ...@@ -470,7 +464,12 @@ uno_threadpool_putJob(
void ( SAL_CALL * doRequest ) ( void *pThreadSpecificData ), void ( SAL_CALL * doRequest ) ( void *pThreadSpecificData ),
sal_Bool bIsOneway ) SAL_THROW_EXTERN_C() sal_Bool bIsOneway ) SAL_THROW_EXTERN_C()
{ {
getThreadPool(hPool)->addJob( pThreadId, bIsOneway, pJob ,doRequest ); if (!getThreadPool(hPool)->addJob( pThreadId, bIsOneway, pJob ,doRequest ))
{
SAL_WARN(
"cppu",
"uno_threadpool_putJob in parallel with uno_threadpool_destroy");
}
} }
extern "C" void SAL_CALL extern "C" void SAL_CALL
......
...@@ -102,7 +102,7 @@ namespace cppu_threadpool { ...@@ -102,7 +102,7 @@ namespace cppu_threadpool {
ThreadAdmin(); ThreadAdmin();
~ThreadAdmin (); ~ThreadAdmin ();
void add( rtl::Reference< ORequestThread > const & ); bool add( rtl::Reference< ORequestThread > const & );
void remove( rtl::Reference< ORequestThread > const & ); void remove( rtl::Reference< ORequestThread > const & );
void join(); void join();
...@@ -126,7 +126,7 @@ namespace cppu_threadpool { ...@@ -126,7 +126,7 @@ namespace cppu_threadpool {
void dispose( sal_Int64 nDisposeId ); void dispose( sal_Int64 nDisposeId );
void destroy( sal_Int64 nDisposeId ); void destroy( sal_Int64 nDisposeId );
void addJob( const ::rtl::ByteSequence &aThreadId, bool addJob( const ::rtl::ByteSequence &aThreadId,
bool bAsynchron, bool bAsynchron,
void *pThreadSpecificData, void *pThreadSpecificData,
RequestFun * doRequest ); RequestFun * doRequest );
...@@ -146,7 +146,7 @@ namespace cppu_threadpool { ...@@ -146,7 +146,7 @@ namespace cppu_threadpool {
ThreadAdmin & getThreadAdmin() { return m_aThreadAdmin; } ThreadAdmin & getThreadAdmin() { return m_aThreadAdmin; }
private: private:
void createThread( JobQueue *pQueue, const ::rtl::ByteSequence &aThreadId, bool bAsynchron); bool createThread( JobQueue *pQueue, const ::rtl::ByteSequence &aThreadId, bool bAsynchron);
ThreadIdHashMap m_mapQueue; ThreadIdHashMap m_mapQueue;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment