Curt Arnold a �crit :
On Aug 13, 2004, at 9:44 AM, Christophe de VIENNE wrote:
Any idea/suggestion welcome,
Not directly on point, but I had a couple of random, unconsidered thoughts related to threads that I'd like to throw out if anyone is going to be digging into to this.
1. Apache Portable Runtime (http://apr.apache.org) provides a C threading API. Thought it probably isn't desirable to require APR especially just for threading, it might be good to support it as a threading option to support rarer platforms.
The threading API of APR looks quite simple (and close to pthread), it shouldn't be a problem to support it the day it's needed.
2. The CriticalSection instance in log4cxx::helpers::ObjectImpl causes config.h to be expanded for the abstract classes in public API. Isolating implementation details from the client API would require removing the CriticalSection from the abstract base class and reintroducing it in the concrete implementation classes. perhaps via templating and/or macros.
Another solution is to typedef the main types we use in threading, depending on PTHREAD (not HAVE_PTHREAD), and WIN32. This will require the library user to add -DPTHREAD on it's compiler command line, which is not a problem I think (a lot of other library do).
For example in portability.h, having:
#ifdef PTHREAD typedef pthread_mutex_t log4cxx_mutex_t; typedef phtread_key_t log4cxx_key_t; typedef pthread_t log4cxx_thread_t; #else #ifdef WIN32 typedef CRITICAL_SECTION log4cxx_mutex_t; typedef void * log4cxx_key_t; typedef void * log4cxx_thread_t; #endif #warning "No threading support" #endif
OR, we keep the HAVE_xx macros, but build them from PTHREAD and WIN32 :
#if defined(PTHREAD) # define LOG4CXX_HAVE_PTHREAD #elif defined(WIN32) # define LOG4CXX_HAVE_MSTHREAD #endif
About TheadSpecificData, I'm thinking of improving the API this way :
class ThreadSpecificData
{
public:
ThreadSpecificData();
ThreadSpecificData(void (*cleanup)(void*)); // this is a new constructor.
void * GetData() const;
void SetData(void * data);// .... };
/** This ptr interface is taken from boost, I don't know if we can take it like this. On the other hand, starting from scratch with the same goal in mind, we'd end with the exact same result... */
template < class T >
class thread_specific_ptr
{
public:
tsd_ptr(): m_impl( &tsd_ptr::cleanup ) {}
T* get() const {
return m_imp->getData();
}T* operator->() const { return get(); }
T& operator*() const { return *get(); }
T* release() {
T* tmp = get();
m_impl.setData(0);
return tmp;
}
operator bool () { return get() == 0; } operator T& () { return *get(); } void reset(T* p=0) {
T* tmp = m_impl.getData();
if(tmp)
delete tmp;
impl_.setData(p);
}
private: static void cleanup(void* p) { delete static_cast<T*>(p); }
ThreadSpecificData m_impl; };
Christophe
