Fix race condition on wait()
#501
Conversation
|
It looks like @foriequal0 hasn't signed our Contributor License Agreement, yet.
You can read and sign our full Contributor License Agreement at the following URL: https://cla.parity.io Once you've signed, please reply to this thread with Many thanks, Parity Technologies CLA Bot |
`wait()` using `oneshot::channel()` makes race conditions. Call `Executor::wait()` directly. It is possible that the thread that has executed `wait()`, which is usually the main thread, terminates before the thread executing `done_tx.send(())` drops `rpc_handler`. Static variables are destructed at the end of termination of the main thread, and then a segfault occurs when the thread dropping the `rpc_handler` accesses the variables.
|
[clabot:check] |
|
It looks like @foriequal0 signed our Contributor License Agreement. 👍 Many thanks, Parity Technologies CLA Bot |
There was a problem hiding this comment.
@foriequal0 thank you for the contribution, the code looks super clean now.
I think we have changed the behavior slightly though - if you want to wait for the server to finish - we only wait for the executor to finish.
I see one issue with the current implementation though: If the event loop is external wait is pretty much a no-op, so we definitely won't wait for the server to finish.
Any ideas how to address that?
|
@tomusdrw I just realized that I can fix this issue with a few lines of changes while not changing the behavior.
Should I revert the commit and proceed with this idea? |
|
I'll close this and create a new PR(#504) with the suggested fix |
wait()usingoneshot::channel()makes race conditions. CallExecutor::wait()directly.It is possible that the thread that has executed
wait(), which is usually the main thread, terminates before the thread executingdone_tx.send(())dropsrpc_handler.Static variables are destructed at the end of termination of the main
thread, and then a segfault occurs when the thread dropping the
rpc_handleraccesses the variables.