Frank,
> o Any given GDALDataset is used only from one thread.
>
>
Does it mean: "GDALDataset can be used only from one thread mutually"?
Or "GDALDataset can be used in one specific (creation?) thread"? Or
worse..? Can we use given dataset in many threads if we synchronize
access to it's methods and objects produced eg. rasterbands?
> o The GDAL driver list should be initialized only once (ie. a
> call to GDALAllRegister()).
>
>
Certainly.
> o Currently error handlers are per-thread, so if you want to install
> a custom error handler, you will need to do this for each thread
> depending on it.
>
>
Not a very convenient solution, especially when an application has
many threads and thread pools. Why error handlers cannot be
process wide?
> o The block cache is shared amoung all threads.
>
>
Does it mean that the cache is thread safe? Can we create a
number of datasets and process them concurrently in various
threads without any explicit locking ? If so, it is a great news :-) !
> o My testing has been mostly in a read-only environment. I don't
> see any obvious problem with writing, but it hasn't been tested
> in a multithreaded environment.
> o I have not reviewed the OGRSpatialReference related code for
> threadsafety yet.
>
In my company we assumed we cannot change PROJ.4, in particular
we cannot neither place *pj*_errno in thread specific storage nor
modify PROJ.4 to return error codes rather than to modify
global variables.
This implied a global PROJ.4 mutex in ogrct.cpp and locking in the
following function/methods:
LoadProjLibrary(),
char *OCTProj4Normalize( const char *pszProj4Src ),
OGRProj4CT::~OGRProj4CT(),
int OGRProj4CT::Initialize( OGRSpatialReference * poSourceIn,
OGRSpatialReference * poTargetIn )
int OGRProj4CT::TransformEx( int nCount, double *x, double *y, double *z,
int *pabSuccess ).
Because all PROJ.4 related methods are enclosed in ogrct.cpp, PROJ.4 lock
has not to bo exposed outside GDAL. This was sufficient to achieve
reentrancy of all OGRSpatialReference related methods in OGR (at least in
our tests :-) )
>For background, I have made a number of additions to cpl_multiproc.cpp/h
>including the addition of a mutex holder class.
>
Great! I made a similiar class when was working on GDAL thread safety
few weeks ago. I even planned to post my changes, but I see I'm too
late :-). Let me share then my multithread experiences to make
GDAL a better software:
1. It is better to separate the creation of the mutex from its locking
(see eg. boost:thread, ACE::ACE_Mutex). These activities are quite
different and it is better to not mix them.
Currently, in GDAL mutexes are locked during creation. It is not very
convenient and GDAL by itself is a proof for this statement. In
1.2.6 mutexes were employed only in GDALwarp::alg, and they were
released just after they were created! Why to make such simple
operations so complex then?
2. Usually it is good to have a class for holding mutex (similiar to
CPLMutexHolder). But this class should be responsible for holding
a mutex only. It should expose and interface for direct
acquire/try_acquire/release, but should not lock a mutex by
itself in constructor (see eg. boost::thread, ACE::ACE_Mutex). In
fact such mutex holder should be a C++ wrapper around
native OS functions.
3. It is good to not use mutexes directly, but via guard class (see eg.
boost::thread, ACE::ACE_Mutex) . The guard class ("the creation
of the object is resource allocation", Stroustrup) should acquire a
mutex when created and release it when destroyed. Additionally,
a guard should expose a release method. In general, in larger
projects it is better to use guards rather than direct
direct locking, as this prevents some deadlocks.
4. To serve specific purposes better, a guard class should have a
copy semantics aka. std::auto_ptr, namely left hand side guard
during copying (copy ctor, assignment operator) should grab
a mutex (disown) from RHS guard. This way, one
may return a guard to synchronize the access to an object and
be (almost) sure the mutex will be released. It is better solution
than exposing a mutex directly.
I've attached an examplary cpl_mutex classes ( which fullfills 1-3
requirements) to this e-mail. If you find it usefull, please consider
using them in GDAL.
>I have also modified the assumptions with regard to CPLAcquireMutex().
>It is now expected to support recursive mutex acquisition. That is, a
>thread can safely acquire an already held mutex again.
>
Good! That was another thing which was not very consistent in
previous versions of GDAL (recursive under win32, but
non-recursive under linux). In general, in OO programing it is
better to use recursive mutexes, though some say that flaws
in architecture manifest sometimes as a need for recursive
mutexes :-)
> Normal reference
>counting rules apply. I am a bit concerned about how portable this will
>be on different pthreads implementations. Currently I have tested it on
>Linux and I had to use the apparently non-standard
>PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP mutex initialization.
>It may be that additional work will be needed for multi-threaded mutex
>support on other platforms.
>
>
Recursive mutexes in pthreads are available in UNIX98 extensions.
Here is the list of platforms, which seem to support recursive
mutexes natively, though not necessary via pthreads:
AIX >=4 (pthread), linux (pthread), cygwin32 (pthread),
freebsd > 4 (pthread), hpux >=11 (pthread), netbsd (pthread),
win32 (win sdk), psos-diab-ppc.
Probably, recursive mutexes are also avalaible in many other platforms
(eg. sunos, qnx nto), but do not want to mention them explicitely
as I'm cannot verify that at this moment. Neverthless, even
if recursive mutex is not avalaible directly one can emulate it.
But to faciliate this it is better to have CPLMutex class rather
than use void** directly :-).
>The other key change was introduction of the CPL_THREADLOCAL
>macro. This attribute for a variable marks it as being thread-local.
>That is, a separate instance will exist for each thread. I have applied
>this to a variety of static buffers in gdal/port such as the "last error
>message" buffer. On win32 this evaluates to __declspec(thread).
>On other platforms it resolves to nothing by default, but if you have
>an appropriately modern version of gcc you can have it expand
>to __thread. I have tested this with gcc 3.3 on Linux successfully.
>I still need to develop the appropriate configuration control around
>this definition.
>
>
Thread specific storage is avalaible on many platforms, but
unfortunatelly one cannot expect that a single macro solves
the problem in portable way. A more serious and portable
attempt is to create some wrappers for accessing thread specific
keys and then to define all those thread variables as a function
calls which get values from thread specific storage. Obviously, such
solution is more advanced, and requires much more work than
simple declspec or __thread. However, it seems that this is the
only way to achieve true portability across many platforms
(eg. see ACE). Do you plan to restrict multithread GDAL to
win32/gcc 3.3 only?
>Those with a strong interest in GDAL multi-threaded support are
>encouraged to test it over the next two weeks while I am away.
>However, as it is all quite new some teething problems are to be
>expected. So take care with the latest CVS version of GDAL.
>
>
I'm really gdal ( :-) ) to see all these changes.
Thank you.
Marek
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cpl_mutex.h
Type: text/x-chdr
Size: 4245 bytes
Desc: not available
Url : http://lists.osgeo.org/pipermail/gdal-dev/attachments/20050531/4ce46969/cpl_mutex.bin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cpl_mutex.cpp
Type: text/x-c++src
Size: 3461 bytes
Desc: not available
Url : http://lists.osgeo.org/pipermail/gdal-dev/attachments/20050531/4ce46969/cpl_mutex-0001.bin