-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PreparedGeometry::contains is not thread safe #95
Comments
Can you provide me a code having this issue so I can fix it and enforce it through tests please? |
https://github.com/gauteh/roaring-landmask/pull/6/files#diff-6423871468d8d2f4ebf2a85e879f9ba388c361b859c9191d62d4571685c95f58 , I think you need a big geometry for this to trigger. |
Ok, I'll try to take a look when I have a bit of time. |
👍 thanks. I can work around this atm by using a warmup call. It is most likely an upstream bug. But would be a pity to !Send/!Sync Geometry/PreparedGeometry just because of that. |
Don't hesitate to take a look in the meantime. If yu can find a way to go around this limitation without removing |
If we can afford a warmup call to `contains`, that is the only way I can
think of at the moment (without fixing upstream). From the backtrace in
geos it seems like the segfault happens somewhere in sorting of a vector in
the geometry. I'll take a further look if(/when..?) I find the time!
…On Wed, Sep 8, 2021 at 8:33 PM Guillaume Gomez ***@***.***> wrote:
Don't hesitate to take a look in the meantime. If yu can find a way to go
around this limitation without removing Send and Sync, it'd be perfect. :)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#95 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAN364WETWQORRMMUE24X3UA6UALANCNFSM5DUZS3CA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
Thanks, greatly appreciated! A bit under the water currently. ^^' |
No problem, thanks for the effort! |
In version 9 with geos 3.11 it is no longer reliable to warm up the prepared geometry to make it thread safe. |
This is still the case, I don't know how to fix this. PreparedGeometry is not thread-safe. This test fails: https://github.com/gauteh/roaring-landmask/blob/main/src/lib.rs#L328 In https://github.com/gauteh/roaring-landmask/blob/geos311/src/shapes.rs the unsafe is removed, and the test is still failing. |
Hi,
It seems that prepared geometry contains is not thread safe before run at least once. Easiest to provoke on large geometries. Not able to provoke on example in docs.
Regards, Gaute
The text was updated successfully, but these errors were encountered: