Opened 14 years ago
Closed 14 years ago
#3628 closed defect (fixed)
race condition in static variable bTLSKeySetup Cpl_multiproc.cpp
Reported by: | lucafibbi | Owned by: | ilucena |
---|---|---|---|
Priority: | normal | Milestone: | 1.8.0 |
Component: | default | Version: | unspecified |
Severity: | normal | Keywords: | thread safe |
Cc: | warmerdam |
Description (last modified by )
In the Cpl_multiproc.cpp file when using the static variable bTLSKeySetup simultaneously from multiple threads race condition occurs.
I think that the following implementation of the functions CPLCleanupTLS and CPLGetTLSList for pthred library should be thread safe: {{ static pthread_once_t oTLSKeySetup = PTHREAD_ONCE_INIT;
static void CPLMake_key() {
if( pthread_key_create( &oTLSKey,
(void (*)(void*)) CPLCleanupTLSList ) != 0 )
{
CPLError( CE_Fatal, CPLE_AppDefined,
"pthread_key_create() failed!" );
}
}
// /* CPLCleanupTLS() */ //
void CPLCleanupTLS()
{
void papTLSList;
papTLSList = (void ) pthread_getspecific( oTLSKey ); if( papTLSList == NULL )
return;
pthread_setspecific( oTLSKey, NULL );
CPLCleanupTLSList( papTLSList );
}
// /* CPLGetTLSList() */ //
static void CPLGetTLSList()
{
void papTLSList;
if ( pthread_once(&oTLSKeySetup, CPLMake_key) != 0 ) {
CPLError( CE_Fatal, CPLE_AppDefined,
"pthread_once() failed!" );
}
papTLSList = (void ) pthread_getspecific( oTLSKey ); if( papTLSList == NULL ) {
papTLSList = (void ) CPLCalloc(sizeof(void*),CTLS_MAX*2); if( pthread_setspecific( oTLSKey, papTLSList ) != 0 ) {
CPLError( CE_Fatal, CPLE_AppDefined,
"pthread_setspecific() failed!" );
}
}
return papTLSList;
} }}}
Change History (5)
comment:1 by , 14 years ago
Cc: | added |
---|---|
Description: | modified (diff) |
Milestone: | 1.8.0 |
Owner: | changed from | to
comment:2 by , 14 years ago
Frank,
I applied those changes (on my local files) but I am not sure how can I test it.
comment:3 by , 14 years ago
Ivan,
Generally speaking you will not be able to trigger such a race condition since the impact is so rare. But you should ensure you are built with pthread support on, and perhaps run the test suite.
comment:4 by , 14 years ago
Frank,
Does the builtbot setting include multi-threads support in Linux and Windows?
Is there a specific test case for multi-threads?
comment:5 by , 14 years ago
Milestone: | → 1.8.0 |
---|---|
Resolution: | → fixed |
Status: | new → closed |
Ivan,
from what I see (http://buildbot.osgeo.org:8500/builders/telascience-full/builds/518/steps/configure/logs/stdio), the Linux slavebots are not configured with --with-threads. (The windows builds of GDAL have always threading turned on however)
test_gdalwarp_18() in autotest/utilities/test_gdalwarp.py will use the -multi option of gdalwarp. Apart from that one, I don't think there are any other test that make use of multi-threading. And I'm 99,9999% sure that it wouldn't exhibit the race condition of that ticket.
The race condition that existed before would have been very hard to reproduce. It could happen only the first time CPLGetTLSList() was called. Basically, you'd need to make a special program that has 2 threads calling CPLGetTLS(). As often with multi-threading issues, only careful code review can really tell if the code is correct or not.
I'd note that the Win32 implementation of CPLGetTLSList() is still subject to a similar race condition that the pthread implementation was. According to http://msdn.microsoft.com/en-us/library/ms885202.aspx, TlsAlloc() would be best called frm a DLL entry point to avoid race conditions. But that'd a different story/ticket
Ivan,
Could you review and if appropriate apply the changes?