<div dir="ltr">+1 KurtS<br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Sep 11, 2024 at 12:27 PM Even Rouault via gdal-dev <<a href="mailto:gdal-dev@lists.osgeo.org">gdal-dev@lists.osgeo.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi,<br>
<br>
I move to approve RFC 101 "Raster dataset read-only thread-safety": <br>
<a href="https://github.com/OSGeo/gdal/pull/10676" rel="noreferrer" target="_blank">https://github.com/OSGeo/gdal/pull/10676</a><br>
<br>
Starting with my +1,<br>
<br>
The candidate implementation is available in <br>
<a href="https://github.com/OSGeo/gdal/pull/10746" rel="noreferrer" target="_blank">https://github.com/OSGeo/gdal/pull/10746</a>. While fine tuning it, I <br>
realized there were subtleties, related to the fact we use thread-local <br>
datasets under the hood, for methods returning a non-"primitive" type, <br>
such as a "const char*" whose lifetime is tied to a dataset/band. If <br>
we'd get it from a thread-local dataset, and that one would be later <br>
evicted from the cache (not very common, but could happen if one would <br>
open tons of thread-safe datasets at the same time), then you could have <br>
use-after-free issues. For such methods (GetMetadata(), <br>
GetMetadataItem(), GetProjectionRef(), etc. as well as GetColorTable() <br>
which returns a GDALColorTable pointer, or GetSpatialRef() which returns <br>
a OGRSpatialReference*), I've preferred to protect their access with a <br>
mutex around the "prototype" dataset passed when constructing <br>
GDALMultiThreadedDataset, whose lifetime is at least as long as <br>
GDALMultiThreadedDataset (cf commit <br>
<a href="https://github.com/OSGeo/gdal/pull/10746/commits/849e9cea711efd30c47fd90a7a6b71a75611c1a5#diff-687008dd6e3d5ac8ad05568272746ed75b84de50bde508f78f2d79a6842825c7L428" rel="noreferrer" target="_blank">https://github.com/OSGeo/gdal/pull/10746/commits/849e9cea711efd30c47fd90a7a6b71a75611c1a5#diff-687008dd6e3d5ac8ad05568272746ed75b84de50bde508f78f2d79a6842825c7L428</a>) <br>
. Normally such methods aren't called in user code repeatedly, so there <br>
should be any noticeable lock contention in practice. The main objective <br>
of the RFC which is to be able to issue RasterIO() requests in parallel <br>
isn't affected by that. Ah, and one thing I realized is that <br>
OGRSpatialReference isn't thread-safe, so I've also added an optional <br>
SetThreadSafe() on it, to also use a per-instance mutex in <br>
multi-threaded scenarios (cf commit <br>
<a href="https://github.com/OSGeo/gdal/pull/10746/commits/c7e1862273dd018e58a01f25b21fdff6dbfdd1cd" rel="noreferrer" target="_blank">https://github.com/OSGeo/gdal/pull/10746/commits/c7e1862273dd018e58a01f25b21fdff6dbfdd1cd</a>). <br>
Multi-threading is hard...<br>
<br>
Even<br>
<br>
-- <br>
<a href="http://www.spatialys.com" rel="noreferrer" target="_blank">http://www.spatialys.com</a><br>
My software is free, but my time generally not.<br>
<br>
_______________________________________________<br>
gdal-dev mailing list<br>
<a href="mailto:gdal-dev@lists.osgeo.org" target="_blank">gdal-dev@lists.osgeo.org</a><br>
<a href="https://lists.osgeo.org/mailman/listinfo/gdal-dev" rel="noreferrer" target="_blank">https://lists.osgeo.org/mailman/listinfo/gdal-dev</a><br>
</blockquote></div>