Fix sync server deadlock v1.2.0#439
Conversation
|
Hi @AndrushaUt, thanks for the PR. Were you able to come up with a way to reliably reproduce #441 ? That would be helpful in reviewing this fix. Could you also restore the deleted |
The shared `_return_queue` causes a race condition when multiple threads call `to_image` concurrently: `task_queue.join()` unblocks all waiters when the count hits zero, not when a specific task completes. Two threads race to `return_queue.get()`, one wins, the other blocks forever. Fix: give each caller its own `Queue` embedded in the `Task`. The server routes the result directly into it. No sharing, no race, no deadlock. Additionally, `_sync` functions now auto-start the singleton server instead of falling back to `oneshot_async_run`, which creates a new thread and event loop per call and can also hang under load. The `atexit` handler that called `close()` → `thread.join()` is removed since the server thread is already `daemon=True` and will exit with the process. The `join()` could hang if Chromium shutdown blocks. Reproduces ~1/1000 calls with 64 threads. See https://github.com/plotly/plotly.py/issues/5549
d6058a1 to
f5426b1
Compare
|
Hi @emilykl ! I've force-pushed the branch - it's now rebased on master with workflows restored and all changes in a single clean commit. With the fix applied, we ran 6000+ calls with zero hangs. |
|
looks similar to #431 |
|
Hi @emilykl - checking in on this. It's been a while since the last update; let me know if there's anything else you'd like me to address or if it's still in the review queue. |
…access
The shared _return_queue causes a race condition when multiple threads call to_image concurrently: task_queue.join() unblocks all waiters when the count hits zero, not when a specific task completes. Two threads race to return_queue.get(), one wins, the other blocks forever.
Fix: give each caller its own Queue embedded in the Task. The server routes the result directly into it. No sharing, no race, no deadlock.
Reproduces ~1/1000 calls with 64 threads.
Closes #441.