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
GeoTools-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel
