bschofield opened a new pull request #663: URL: https://github.com/apache/pulsar-client-go/pull/663
Fixes #662 ### Motivation As described in #662, there appears to be a potential race condition in connection.go function _waitUntilReady()_: the `cond.Broadcast()` can occur before the `cond.Wait()`. ### Modifications Function `waitUntilReady()` was previously holding the global `c.Lock()` on the connection. From my reading of the code, this mutex is intended to protect the `cnx` variable. I think that the use of `c.Lock()` in `waitUntilReady()` was probably just a typo. Instead, I think the author probably intended to grab the lock associated with the `sync.Cond`, i.e. `c.cond.L.Lock()`. This looks like the correct thing to do when using a `sync.Cond`. I think there should be a corresponding lock around the `cond.Broadcast()` also. See https://stackoverflow.com/questions/36857167/how-to-correctly-use-sync-cond/42772799#42772799 for more details. ### Verifying this change I am unsure if this change is covered by existing tests. It fixes a rare race condition, so I think it would be quite difficult to test for. I have deployed this change on my own production system, and it doesn't obviously break anything. I report back if I see any issues that might be related to it. ### Does this pull request potentially affect one of the following parts: - Dependencies (does it add or upgrade a dependency): no - The public API: no - The schema: no - The default values of configurations: no - The wire protocol: no ### Documentation - Does this pull request introduce a new feature? no - If yes, how is the feature documented? not applicable -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
