search

neo4j / neo4j-go-driver

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

Fix #538 #540

Closed DaChartreux closed 8 months ago

DaChartreux commented 8 months ago

This PR is a fix for issue #538

Suppose there are 2 goroutines A and B. Assume A is updating the routing table. While this happening, B comes and sees that someone is already updating the routing table, and appends its channel to get notified once A updates the table.

Happy case

If B's context is not Done(), then A will send an empty struct{} to the channel and then B continues with its flow.

Failing case

If B's context is done before A updates the routing table, then B stops listening to the channel it just pushed to be notified on. Now once A updates the table, it goes to send empty struct{} to the channel, but this results in a panic situation because there is no one to listen on that channel anymore.

Fix

Instead of notifying the goroutines by sending an empty struct{}, we should rather just close the channel. Closing the channel is panic safe, and will continue on with the existing flow.

DaChartreux commented 8 months ago

@StephenCathcart I have created a new PR with the user with which I signed the CLA. Please check now?

DaChartreux commented 8 months ago

@StephenCathcart Why is it failing now? 🤔

StephenCathcart commented 8 months ago

@StephenCathcart Why is it failing now? 🤔

Hi, we have an extra security feature, a membership check, as part of our build which was failing but has now been resolved (the build is currently running). Your whitelist check is ok. I'll let you know once this has been merged and will be included in the upcoming release at the end of the month.