Thread Sanitizer warns of data race in geos::util::Interrupt::cancel()

[macdrevx]

Original ticket: ​https://github.com/GEOSwift/GEOSwift/issues/192

Actual geos version is 3.7.1 but it's not one of the available options in Version field.

Two threads invoke GEOS_init_r() which in turn invokes geos::util::Interrupt::cancel(). Both threads write to the requested variable, which is detected by the [Thread Sanitizer|​https://developer.apple.com/documentation/code_diagnostics/thread_sanitizer] as a data race.

[dbaston]

Both threads write the same value, however. Seems like the bigger issue would be if one thread is checking for an interrupt while another thread is requesting one.

Could change the variable in question to a std::atomic<bool>.

[macdrevx]

If I'm reading the source correctly, it seems like the interrupt mechanism can be used from any thread and when it is, it will interrupt only that one thread. Is there any reason why this shouldn't be a thread-specific or context-specific construct?

[macdrevx]

I've spent some time considering what would be gained by making requested atomic. While it would avoid the data race on that particular memory, it would not result in a thread-safe system overall.

Consider this example: there are long-running operations on 2 threads. A 3rd thread requests an interrupt. The result could be either:

1. 1 thread is interrupted (passes the check at Interrupt.cpp:66 and executes line 67 to lock out the other thread), or
2. Both threads are interrupted. (Both threads pass the check at Interrupt.cpp:66 before either one executes line 67).

Which scenario occurs (and within scenario A, which thread is interrupted) depends on the timing.

A better solution would provide an interface that allows the consumer to be specific about their intent. My current thought is that for the thread safe CAPI we could design a new interface that would allow requesting an interrupt only on a specific context. Ideally, any such interface would avoid requiring the CAPI consumer to implement synchronization mechanisms around the context. Optionally, we could maintain the existing GEOS_interruptRequest interface to allow requesting interruption on all contexts.

I have some implementation ideas, but I'll wait to share them until after getting some feedback about whether this general direction is desirable.

[robe]

Retargeting in prep for GEOS 3.10.0 release

[dbaston]

A thread-specific interruption mechanism is proposed in ​https://github.com/libgeos/geos/pull/761