Fix nested ASIO streams causing deadlock on drop - #1313
Conversation
0940101 to
4cdffb9
Compare
swap_remove reorders the vec, breaking add_callback's bc.last().id + 1 scheme and causing duplicate BufferCallbackIds. remove preserves order while still dropping the removed callback after the lock is released.
bcs.clear() dropped every registered callback in place while still holding the lock. If a callback owned another stream, dropping it here would reenter remove_callback and deadlock on the same lock.
Weak::upgrade() in load_driver returns None as soon as the old DriverInner's Arc strong count hits zero, which happens before DriverInner::drop (and therefore ASIOExit) has finished running. A second thread could then start ASIOInit before the old driver's ASIOExit had returned. Share loaded_driver's lock with DriverInner so destroy_inner holds it across ASIOExit, same as load_driver already does across ASIOInit.
buffer_switch_time_info held BUFFER_CALLBACK across running every registered callback. If a callback synchronously dropped a Stream it owned, that Stream's Drop would call remove_callback and try to re-lock BUFFER_CALLBACK on the same thread, deadlocking on the ASIO real-time callback thread. Track whether the current thread is inside buffer_switch_time_info via a thread-local flag; remove_callback checks it and, if set, defers the removal into a thread-local queue instead of blocking on the lock. buffer_switch_time_info drains that queue after running callbacks, still under the lock, before releasing it.
|
It's great that you and @edwloef showed this works. Reviewing it though, I see that swapping Going through the stream lifecycle, when one callback holding a nested stream, I think there's more issues:
I'd appreciate your local testing on this. |
…r-load race fixes
|
|
|
Yeah I wouldn't mind removing it either. Let me know if you or @LastExceed not running into that when you tear down mid-callback. |
|
What exactly do you want me to test? |
3 participants
Reported by @edwloef (on Discord):
A curious configuration, but alas. Fixing it was a simple as changing the drop order.