Opened 6 years ago
Closed 17 months ago
#855 closed defect (fixed)
Race condition in Geometry::getEnvelopeInternal
Reported by: | namr | Owned by: | strk |
---|---|---|---|
Priority: | major | Milestone: | 3.8.2 |
Component: | Core | Version: | 3.6.2 |
Severity: | Significant | Keywords: | Geometry::getEnvelopeInternal |
Cc: |
Description
When getting the intersection of two geometries, if they were not previously generated, Geos builds the Envelope for both of those. However, in a heavy multi-threaded environment, when caching the geometries, multiple threads access and generate the envelope, creating a race condition which produces a SEGFAULT. A temporary solution is to call the method getEnvelope at load time, to pre-compute it...
Attachments (1)
Change History (10)
comment:1 by , 6 years ago
comment:2 by , 6 years ago
Milestone: | 3.6.3 → 3.8.0 |
---|
comment:4 by , 5 years ago
Resolution: | → worksforme |
---|---|
Status: | new → closed |
comment:5 by , 4 years ago
Resolution: | worksforme |
---|---|
Status: | closed → reopened |
I have encountered the same problem in GEOS 3.8.0. I've attached some test code that segfaults about 60% of the time for me under Linux. As noted by namr, it's possible to workaround the problem by calling GEOSEnvelope_r before performing any multithreaded calculations.
comment:6 by , 4 years ago
Thank you for the test code. Technically the GEOS "thread-safe" API requires a separate context for each thread, which would resolve this problem. But it would also severely the scope of operations that can be run using multiple threads. I think it's worth addressing.
A relatively small fix is to set a mutex on whatever part of the the lazy-envelope calculation is causing the segfault.
A larger fix is to rethink the approach of lazy envelope calculation. Given that (a) geometries almost always require an envelope, and (b) envelopes can be most efficiently calculated when creating a coordinate sequence, it seems like it might make sense to eagerly calculate envelopes whenever a coordinate sequence is created. I did some experimentation along these lines a while ago; can't remember what the status of it is.
In either case, the larger fix would be something for 3.9.
comment:7 by , 4 years ago
Thanks for your response. The requirements for using the thread-safe API weren't clear to me but I had concluded that it requires a different context for each thread (and I have done this in the test code). But is it also necessary to avoid sharing geometries between threads?
comment:8 by , 4 years ago
Milestone: | 3.8.0 → 3.8.2 |
---|
comment:9 by , 17 months ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
This should be resolved with 62edc7f05d44e83c1e70e1bf4e7f91738061d982/git
Thanks, GEOS (and JTS) are full of those small things, we never added any thread synchronization in the code. I'm happy to accept patches to start with it. Maybe look at c++11, the more standard solution we adopt the easier to maintain in the long run.