[gdal-dev] Motion: approve RFC 101 "Raster dataset read-only thread-safety"

[Even Rouault]

Hi,

I move to approve RFC 101 "Raster dataset read-only thread-safety": 
https://github.com/OSGeo/gdal/pull/10676

Starting with my +1,

The candidate implementation is available in 
https://github.com/OSGeo/gdal/pull/10746. While fine tuning it, I 
realized there were subtleties, related to the fact we use thread-local 
datasets under the hood, for methods returning a non-"primitive" type, 
such as a "const char*" whose lifetime is tied to a dataset/band. If 
we'd get it from a thread-local dataset, and that one would be later 
evicted from the cache (not very common, but could happen if one would 
open tons of thread-safe datasets at the same time), then you could have 
use-after-free issues. For such methods (GetMetadata(), 
GetMetadataItem(), GetProjectionRef(), etc. as well as GetColorTable() 
which returns a GDALColorTable pointer, or GetSpatialRef() which returns 
a OGRSpatialReference*), I've preferred to protect their access with a 
mutex around the "prototype" dataset passed when constructing 
GDALMultiThreadedDataset, whose lifetime is at least as long as 
GDALMultiThreadedDataset (cf commit 
https://github.com/OSGeo/gdal/pull/10746/commits/849e9cea711efd30c47fd90a7a6b71a75611c1a5#diff-687008dd6e3d5ac8ad05568272746ed75b84de50bde508f78f2d79a6842825c7L428) 
. Normally such methods aren't called in user code repeatedly, so there 
should be any noticeable lock contention in practice. The main objective 
of the RFC which is to be able to issue RasterIO() requests in parallel 
isn't affected by that. Ah, and one thing I realized is that 
OGRSpatialReference isn't thread-safe, so I've also added an optional 
SetThreadSafe() on it, to also use a per-instance mutex in 
multi-threaded scenarios (cf commit 
https://github.com/OSGeo/gdal/pull/10746/commits/c7e1862273dd018e58a01f25b21fdff6dbfdd1cd). 
Multi-threading is hard...

Even

-- 
http://www.spatialys.com
My software is free, but my time generally not.