Race condition with GC stopping the world and framework_peer_release_lock

[rolfbjarne]

Deadlock: https://gist.github.com/Therzok/6132ea63a97eb2bcc7b9d7282e973f72

Thread 29 is trying to stop the world:

  thread #29
    frame #0: 0x00007fff6efd8246 libsystem_kernel.dylib`semaphore_wait_trap + 10
    frame #1: 0x000000010c27b978 libmonosgen-2.0.dylib`mono_os_sem_timedwait [inlined] mono_os_sem_wait at mono-os-semaphore.h:90 [opt]
    frame #2: 0x000000010c27b970 libmonosgen-2.0.dylib`mono_os_sem_timedwait(sem=<unavailable>, timeout_ms=<unavailable>, flags=MONO_SEM_FLAGS_NONE) at mono-os-semaphore.h:109 [opt]
    frame #3: 0x000000010c27b1f5 libmonosgen-2.0.dylib`mono_threads_wait_pending_operations at mono-threads.c:310 [opt]
    frame #4: 0x000000010c21ec33 libmonosgen-2.0.dylib`sgen_client_stop_world at sgen-stw.c:297 [opt]
    frame #5: 0x000000010c21ec1f libmonosgen-2.0.dylib`sgen_client_stop_world(generation=1, serial_collection=0) at sgen-stw.c:124 [opt]
    frame #6: 0x000000010c231f76 libmonosgen-2.0.dylib`sgen_stop_world(generation=1, serial_collection=0) at sgen-gc.c:3821 [opt]

Thread 1 is trying to lock framework_peer_release_lock:

* thread #1, name = 'GUI Thread', queue = 'com.apple.main-thread'
  * frame #0: 0x00007fff6efe1a46 libsystem_kernel.dylib`__psynch_mutexwait + 10
    frame #1: 0x00007fff6f1a9b9d libsystem_pthread.dylib`_pthread_mutex_lock_wait + 83
    frame #2: 0x00007fff6f1a74c8 libsystem_pthread.dylib`_pthread_mutex_lock_slow + 253
    frame #3: 0x00000001094b75e4 libxammac.dylib`xamarin_framework_peer_lock + 20
    frame #4: 0x00000001094b9a66 libxammac.dylib`xamarin_release_managed_ref + 166

Thread 29 has the framework_peer_release_lock:

(lldb) x/10gx &framework_peer_release_lock
0x1094d9ae0: 0x000000004d555458 0x0001206800000000
0x1094d9af0: 0x4d55545800000000 0x000000000001623a
0x1094d9b00: 0x000ede00000ee003 0xffffffffffffffff
0x1094d9b10: 0xfffffffef6b2651f 0x4d5554584d555458
0x1094d9b20: 0x00007fe2a1aaf5e0 0x00007fe2a1aaf4e0

This means the lock is held by the thread with id 0x1623a, which is thread #29:

thread #29: tid = 0x1623a, 0x00007fff6efd8246 libsystem_kernel.dylib`semaphore_wait_trap + 10, name = 'Thread Pool Worker'

The deadlock goes like this:

1. Thread 29 starts stopping the world, and calls XM's gc_event_callback, which will lock framework_peer_release_lock: https://github.com/xamarin/xamarin-macios/blob/2ee29f38ce955aff07a5025fde296b5fea1a967f/runtime/runtime.m#L855-L870.
2. Thread 29 takes the framework_peer_release_lock, but has not gotten around to stopping the entire world quite yet.
3. Thread 1 tries to take the framework_peer_release_lock, can't, and starts waiting for it to be released.
4. Thread 29 now tries to suspend all threads, and keeps waiting for thread 1 to do so.
5. Deadlock between threads 1 & 29, they're waiting for each other.

[rolfbjarne]

I would think that a potential solution would be to change MONO_ENTER_GC_SAFE here to actually do something for Xamarin.Mac: https://github.com/xamarin/xamarin-macios/blob/2ee29f38ce955aff07a5025fde296b5fea1a967f/runtime/runtime.m#L343-L351

since it's currently a no-op for anything but watchOS: https://github.com/xamarin/xamarin-macios/blob/2ee29f38ce955aff07a5025fde296b5fea1a967f/runtime/xamarin/runtime.h#L295-L300

With this fix the deadlock would be solved, because in step 4 thread 29 wouldn't wait for thread 1.

[rolfbjarne]

@luhenry @kumpera: can you have a look at this and see if my suggestion in the previous comment makes sense?

If so, can I use the same mono API for MONO_ENTER_GC_SAFE as we do for watchOS?

https://github.com/xamarin/xamarin-macios/blob/2ee29f38ce955aff07a5025fde296b5fea1a967f/runtime/xamarin/runtime.h#L318-L321

[Therzok]

INFO [2018-08-29 12:04:00Z]: Initializing Runtime Mono 5.19.0.201
INFO [2018-08-29 12:04:00Z]: Using Xamarin.Mac 4.4.1.178 (master / eeaeb7e)

[lambdageek]

Yeah, with hybrid suspend, those macros shouldn't be noops anymore

[kumpera]

@lambdageek Is this a hybrid suspend or async suspend bug?

It's not clear at all from the discussion

[lambdageek]

If it's 5.19 on desktop, it's hybrid suspend.

@Therzok @rolfbjarne In 2018-08 (5.18) and master (5.19) we turned on hybrid suspend by default on desktop platforms (macOS, Linux/x86+amd64, Windows); preemptive suspend continues to be the default for devices (and as usual for watchOS uses coop). Please ping me if you experience deadlocks. As a short-term workaround set MONO_THREADS_SUSPEND=preemptive to go back to pre-5.18 behavior.

Hybrid suspend:

• Threads in GC Safe mode are preemptively suspended
• Threads in GC Unsafe mode are cooperatively suspended at safepoints
• The mono API functions listed in xamarin-macios/runtime/exports.t4 have all been updated in Mono to perform a transition to GC Unsafe on entry. That means client threads can generally hang out in GC Safe mode (the default for PInvokes).
• I'm aware of a deadlock with Grand Central Dispatch idle threads that previously executed a managed task - they incorrectly get left in GC Unsafe mode and Mono hangs waiting for them to suspend at the next GC. Fix will need an update to the static registrar and to Mono.

[rolfbjarne]

I haven't seen this in 5 years, so closing.