-
Notifications
You must be signed in to change notification settings - Fork 0
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
Fix Several Race Conditions #16
base: gridx_extensions
Are you sure you want to change the base?
Conversation
…tem is shutting down and still tries to send an error on a channel that has been closed.
…cts too fast ends up blocking the dispatchers messagePump until a timeout (30s) happens.
Pull Request Test Coverage Report for Build 4041955883
💛 - Coveralls |
case <-cs.errC: | ||
// channel is closed, don't send the error | ||
default: | ||
cs.errC <- err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally it's problematic if you have a Goroutine closing a channel that another Goroutine can write to. Not too familiar with the rest of the code anymore, but is there any reason we need to close errC
at all? It's mostly used for logging, right? Then I don't see a reason why we can't just keep it open. Writers could just do a select
and throw the error away if the reader Goroutine already stopped.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good idea 🤔 I'm also not familiar with it, I just fixed the issues :D
how would the writer know if the readers goroutine has stopped?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The write to the channel would block, but you can just wrap it in a select
with a default
case to prevent it from causing an issue
When testing the flakyness of our OCPP unit tests we found a few races happening when reconnection of OCPP clients happens really fast.
Bugs
Websocket: Panic Send on closed channel
Websockets error channel should only be used when the channel is not closed already (1f0c95e)
When shutting down Central System, errors are sent on a closed channel
When shutting down Central System, the error channel might be closed right before we try to send something on it. This is checked before now and protected with a lock. (09198de)
Dispatcher Disconnect Handling
When a client disconnects and connectrs really fast, the dispatcher is locked in a state where he is waiting for a request to be completed. The request got removed from the queue though and thus, the dispatcher is not able to find a matching request when the real client answers to the message. A cleaner way to handle the disconnection of clients was proposed. (5bb5806)
Dispatcher nil reference when peeking element from queue
Fix nil reference when the queue peeks a nil element (ddc83d7)