We do not claim our data objects are thread safe (indeed you can see them being duplicated a fair bit in our code to preserve thread safety).
Your pull request would be welcome, we would still need to look carefully at DefaultFeature implementation before considering even the gitters thread safe. -- Jody Garnett On Wed, 13 Feb 2019 at 23:40, Patrick Valsecchi < [email protected]> wrote: > Hello, > > While investigating a bug [1] in MapFish print, I've found what I consider > a bug in DefaultFeatureCollection.getBounds method. > > Maybe I'm wrong, but I usually assume that a getter is thread safe at the > instance level as long as the instance is not modified by another thread. > So I was a bit surprise to see the implementation of this method in the > DefaultFeatureCollection class: > > public ReferencedEnvelope getBounds() { > if (bounds == null) { > bounds = new ReferencedEnvelope(); > > ... computations > > } > return bounds; > } > > Imagine two threads. T1 is stopped just after the allocation+assignement > but before the computations and T2 gets the CPU at this moment. T2 will > return the newly allocated bounds and continue its life with an empty > bounds until T1 resumes. > > Are you interested in a PR on this issue or is the non-thread safeness of > getters accepted in GeoTools? I was thinking of a fix along those lines: > > public ReferencedEnvelope getBounds() { > if (bounds == null) { > ReferenceEnvelope result = new ReferencedEnvelope(); > > ... computations > > bounds = result; > > } > return bounds; > } > > With that, the worst case would be multiple re-computations of the bounds. > But the results would all be correct. > > Looking at other implementations of this method, I can see that > > - ListComplexFeatureCollection's is syncronized (a bit overkill IMHO) > - ListFeatureCollection is along the same lines as my proposed > structure > - TreeSetFeatureCollection is having the same bug as > DefaultFeatureCollection > - The rest is OK (no cache) > > Thanks and CU > > > [1] https://github.com/mapfish/mapfish-print/pull/820 > _______________________________________________ > GeoTools-Devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/geotools-devel >
_______________________________________________ GeoTools-Devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/geotools-devel
