New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Handle possible race condition when using ExecutorEventEmitter #76
Comments
Thanks for the report! I'll try to take a closer look this weekend. |
|
(it might not matter, it sounds like the move is to make the base EE thread-safe anyhow) |
Yeah might be just as well to handle it in the BaseEmitter. I'll see if I can get some spare time and clobber together a suggestion... |
mulling this over, I think it might not be a locking issue - like it sounds like what's happening is:
so while I could probably add a lock to the BaseEventEmitter around some of the fiddly bits (that's a good idea regardless), I think the answer here is to make the once handlers robust against the handler being removed out-of-band prior to the once's cleanup, and/or have the once remove its own listener prior to the async handler completing. something like that. |
Yes that's pretty much what I think is happening as well. I see two ways of making it more robust, One: the simple try / except above Edit: Since you've got much more insight than me in the architecture here, maybe you have a better idea? |
Yeah no, we're of similar minds here. I mulled over making sure the remove runs on the same tick as the trigger, rather than waiting for the callback to complete - I think this is more correct anyway and might ninja it in there, as a digression - but you should still be able to trigger this for a synchronous call. |
We've had an error report in Mycroft with an exception when removing a once handler.
Been trying to reproduce and I think it's a race condition. We have a sequence which sets up a
once
listener waits for 5 seconds for the listener to be called. If the handler is called it returns immediately if the 5 seconds passes without the handler being triggered the listener is removed.Sometimes (quite rarely) the handler gets called, 5 seconds times out and removes the listener. THEN the once wrapper tries to remove the listener.
A try/except block may be a simple solution to remove the exception log
A proper solution would be to have a lock around the critical section(s)
The text was updated successfully, but these errors were encountered: