search

neo4j / neo4j-go-driver

Neo4j Bolt Driver for Go
Apache License 2.0
482 stars 68 forks source link

Fix #538 #539

Closed DaChartreux closed 8 months ago

DaChartreux commented 8 months ago

In method getOrUpdate, if a goroutine is waiting for the routing table to be updated by that one goroutine which is updating the routing table, it pushes a channel into updating map. But when the goroutine reaches its deadline (or times-out), and the goroutine which was updating the table tries to send struct{}{} to the channel, there is no goroutine there to receive and this causes deadlock. Instead of pushing empty struct, we should be closing the channel instead. Which is safe of panics or deadlocks.

StephenCathcart commented 8 months ago

Hi @DaChartreux

Thank you for taking the time to contribute to a Neo4j project, we appreciate all community engagement. Before we review a change we require that users have signed our Contributor License Agreement(CLA). This allows the "whitelist-check (pull-request)" part of our build process to pass. For more details on signing our CLA please see: https://neo4j.com/developer/cla/. Specifically the 'How to Sign' section.

StephenCathcart commented 8 months ago

Hi @DaChartreux, I've received your CLA and added you to our whitelist database. My colleagues and I reviewed the PR and are fairly confident this will fix the bug you're experiencing. We're just doing some further testing around this before merging the fix. I'll keep you up to date with how this is progressing.

Thanks again for your contribution!

DaChartreux commented 8 months ago

Hi @StephenCathcart! Thanks for the whitelist. Let me know if there are any inputs.

StephenCathcart commented 8 months ago

Hi @DaChartreux, our whitelist-check step is still failing. After a bit of investigation, it seems this is due to the author of the actual commits in the PR (Amit Parameshwar) not matching the name that was whitelisted (DaChartreux) in our database.

If possible, would you be able to either get the other identity (Amit) to also sign the CLA, or create a new PR with the now whitelisted DaChartreux identity as the committer which should pass? Apologies for the inconvenience.