Race Condition in GEOS in v6

[ssokol]

Just upgraded to v6 in an attempt to placate the Thread Sanitizer, which was finding race conditions in GEOS in v4. I seem to still be hitting them in v6 - not sure if this is something specific to my code or if there are still issues with the limited use of GEOS in the new version.

Specifically, the issue appears to be happening in the geos::util::Interrupt::cancel() method. Here's the full dump from the sanitizer:

WARNING: ThreadSanitizer: data race (pid=91811)
  Write of size 1 at 0x000109cba640 by thread T45:
    #0 geos::util::Interrupt::cancel() Interrupt.cpp:36 (geos:x86_64+0x43fde8)
    #1 GEOS_init_r geos_ts_c.cpp:342 (geos:x86_64+0x3b9e0b)
    #2 $s8GEOSwift11GEOSContextCACyKcfc GEOSContext.swift:8 (GEOSwift:x86_64+0x2d806)
    #3 $s8GEOSwift11GEOSContextCACyKcfC GEOSContext.swift (GEOSwift:x86_64+0x2d5ff)
    #4 $s8GEOSwift24WKTInitializableInternalPAAE3wktxSS_tKcfC WKTInitializable.swift:12 (GEOSwift:x86_64+0x53c6a)
    #5 $s13FlightBox_PFD13FBNexradBlockC8overlaps3boxSbSo9GLMapBBoxV_tF FBNexradBlock.swift:161 (FlightBox PFD:x86_64+0x100bc4640)
    #6 $s13FlightBox_PFD17MFDViewControllerC16updateRadarLayer3nrbyAA13FBNexradBlockC_tF MFDViewController.swift:2388 (FlightBox PFD:x86_64+0x100e72393)
    #7 $s13FlightBox_PFD14FBRadarFactoryC19handleRegionalBlock33_1A1A86C987D191B4610B1A75BB833973LL2nd4fisbyAA08FBNexradH0C_AA10FBFisbDataCtF FBNexrad.swift:175 (FlightBox PFD:x86_64+0x100c9b7a3)
    #8 $s13FlightBox_PFD14FBRadarFactoryC17processFisbReport33_1A1A86C987D191B4610B1A75BB833973LL4textySS_tF FBNexrad.swift:157 (FlightBox PFD:x86_64+0x100c9aeba)
    #9 $s13FlightBox_PFD14FBRadarFactoryC26websocketDidReceiveMessage6socket4textyAA8FBSocketC_SStFyycfU_ FBNexrad.swift:83 (FlightBox PFD:x86_64+0x100c970f2)
    #10 $s13FlightBox_PFD14FBRadarFactoryC26websocketDidReceiveMessage6socket4textyAA8FBSocketC_SStFyycfU_TA <compiler-generated> (FlightBox PFD:x86_64+0x100c971af)
    #11 $sIeg_IeyB_TR <compiler-generated> (FlightBox PFD:x86_64+0x10001fba0)
    #12 +[ObjC catchException:error:] ObjC.m:7 (FlightBox PFD:x86_64+0x100001ef9)
    #13 $s13FlightBox_PFD14FBRadarFactoryC26websocketDidReceiveMessage6socket4textyAA8FBSocketC_SStF FBNexrad.swift:82 (FlightBox PFD:x86_64+0x100c96c33)
    #14 $s13FlightBox_PFD14FBRadarFactoryCAA19FBWebSocketDelegateA2aDP26websocketDidReceiveMessage6socket4textyAA8FBSocketC_SStFTW <compiler-generated> (FlightBox PFD:x86_64+0x100ca1038)
    #15 $s13FlightBox_PFD11FBTCPSocketC13handleMessage3msgySS_tFyycfU_ FBTCPSocket2.swift:376 (FlightBox PFD:x86_64+0x100a20ea5)
    #16 $s13FlightBox_PFD11FBTCPSocketC13handleMessage3msgySS_tFyycfU_TA <compiler-generated> (FlightBox PFD:x86_64+0x100a211f7)
    #17 $sIeg_IeyB_TR <compiler-generated> (FlightBox PFD:x86_64+0x10001fba0)
    #18 __tsan::invoke_and_release_block(void*) <null>:20370272 (libclang_rt.tsan_iossim_dynamic.dylib:x86_64+0x6a89b)
    #19 _dispatch_client_callout <null>:20370272 (libdispatch.dylib:x86_64+0x2d47)

  Previous write of size 1 at 0x000109cba640 by thread T50:
    #0 geos::util::Interrupt::cancel() Interrupt.cpp:36 (geos:x86_64+0x43fde8)
    #1 GEOS_init_r geos_ts_c.cpp:342 (geos:x86_64+0x3b9e0b)
    #2 $s8GEOSwift11GEOSContextCACyKcfc GEOSContext.swift:8 (GEOSwift:x86_64+0x2d806)
    #3 $s8GEOSwift11GEOSContextCACyKcfC GEOSContext.swift (GEOSwift:x86_64+0x2d5ff)
    #4 $s8GEOSwift19GeometryConvertiblePAAE23evaluateBinaryPredicate33_548B91C3597A5AB2DA24C29309949B6DLL_4withSbs4Int8Vs13OpaquePointerV_A2JtXE_AaB_ptKF GeometryConvertible+GEOS.swift:88 (GEOSwift:x86_64+0x25399)
    #5 $s8GEOSwift19GeometryConvertiblePAAE8overlapsySbAaB_pKF GeometryConvertible+GEOS.swift:128 (GEOSwift:x86_64+0x26e81)
    #6 $s13FlightBox_PFD17FBAirspaceFactoryC31findAirspacesRelativeToLocation5pointSayAA0D0CGSo10CLLocationC_tF FBAirspace.swift:465 (FlightBox PFD:x86_64+0x100cd17d0)
    #7 $s13FlightBox_PFD17FBAirspaceFactoryC23checkAirspacesAndNotify33_FEE8F2CABC0974AED7DFA8432B44A341LLyyF FBAirspace.swift:177 (FlightBox PFD:x86_64+0x100cc1f72)
    #8 $s13FlightBox_PFD17FBAirspaceFactoryC14locationUpdate12notificationySo14NSNotificationC_tFyycfU_ FBAirspace.swift:161 (FlightBox PFD:x86_64+0x100cc12dd)
    #9 $s13FlightBox_PFD17FBAirspaceFactoryC14locationUpdate12notificationySo14NSNotificationC_tFyycfU_TA <compiler-generated> (FlightBox PFD:x86_64+0x100cc132d)
    #10 $sIeg_IeyB_TR <compiler-generated> (FlightBox PFD:x86_64+0x10001fba0)
    #11 __tsan::invoke_and_release_block(void*) <null>:20370272 (libclang_rt.tsan_iossim_dynamic.dylib:x86_64+0x6a89b)
    #12 _dispatch_client_callout <null>:20370272 (libdispatch.dylib:x86_64+0x2d47)

  Location is global '(anonymous namespace)::requested' at 0x000109cba640 (geos+0x0000010b0640)

  Thread T45 (tid=3590663, running) is a GCD worker thread

  Thread T50 (tid=3590770, running) is a GCD worker thread

SUMMARY: ThreadSanitizer: data race Interrupt.cpp:36 in geos::util::Interrupt::cancel()
==================
ThreadSanitizer report breakpoint hit. Use 'thread info -s' to get extended information about the report.

Any suggestions?

Thanks!

[macdrevx]

Looks like both threads are creating a new GEOS context. My understanding is that this is supposed to be thread-safe.

GEOS_init_r is used to create a new context. It invokes geos::util::Interrupt::cancel() which sets a variable named requested to false. This variable is shared across threads. There's actually a comment that states /* Could these be portably stored in thread-specific space ? */. We'll need to check with the geos folks to see if we can get some more clarity on the impact of this concurrent access and what the impact might be to your program.

[macdrevx]

Opened a ticket with the geos project: https://trac.osgeo.org/geos/ticket/1012

[macdrevx]

@ssokol I have some good news and some bad news. First, the bad news:

Our understanding of the root cause of the data race is correct. Multiple threads modify a shared value without synchronization. This would have to be fixed in GEOS. The impact is that if multiple threads try to use the interrupt mechanism simultaneously, there could be problems.

The good news is that GEOSwift doesn't ever request interrupts, so in practice, I think you should be okay apart from the annoyance of having to see these warnings. If we ever had a need to expose the ability to cancel long-running operations, then this would definitely be an issue.

I'll continue discussing the issue with the GEOS folks to see if we can come up with a solution for a future release of GEOS.

[ssokol]

Thanks for the update. I believe you are correct - I've not actually seen a crash that I can directly link to this. Please let me know if I can provide any additional information to you or to the GEOS team. I'll be happy to test updates as they become available.

[macdrevx]

I'm going to go ahead and close this since there's no immediate risk caused by this issue. Hopefully a future version of GEOS resolves it.

[ssokol]

As an FYI - I'm also getting a race condition on the addRef() function in GeometryFactory.cpp. Looks like the read call came from Polygon.geosObject(with:) while a write call happened when a polygon was created from WKT. No crash - just the warning from the sanitizer.

[macdrevx]

@ssokol can you share the stack traces for those too? We can open another issue with GEOS