Fix RPCClient Deadlock on Unsubscribe and NewHead #14236
Conversation
|
| @@ -130,6 +131,33 @@ func TestRPCClient_SubscribeNewHead(t *testing.T) { | |||
| assert.Equal(t, int64(0), highestUserObservations.FinalizedBlockNumber) | |||
| assert.Equal(t, (*big.Int)(nil), highestUserObservations.TotalDifficulty) | |||
| }) | |||
| t.Run("Concurrent Unsubscribe and onNewHead calls do not lead to a deadlock", func(t *testing.T) { | |||
| const numberOfAttempts = 1000 // need a large number to increase the odds of reproducing the issue | |||
There was a problem hiding this comment.
If this only reproduces the issue probabilistically, does that make the test flaky? Or is it just that there will be a low chance of it PASS'ing with a false positive?
There was a problem hiding this comment.
probably just creating a potential race condition, and now we have the right locking, we don't expect the test to fail/deadlock happens. So it shouldn't be flaky I think
There was a problem hiding this comment.
@reductionista, flaky behavior will only occur if there is regression. 1000 attempts seem to be large enough. Before the fix, the test failed on each run.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
* Fix RPCClient Deadlock on Unsubscribe and NewHead * changeset (cherry picked from commit 0294e1f)
* Fix RPCClient Deadlock on Unsubscribe and NewHead * changeset (cherry picked from commit 0294e1f)
Cherry-pick of smartcontractkit/chainlink#14236 --------- Co-authored-by: Dmytro Haidashenko <34754799+dhaidashenko@users.noreply.github.com>
Cherry-pick of smartcontractkit/chainlink#14236 --------- Co-authored-by: Dmytro Haidashenko <34754799+dhaidashenko@users.noreply.github.com>
Ticket
Concurrent calls to
onNewHeadandUnsubscribeAllExceptAliveLoopcaused deadlock onstateMu. As a result, any call to any configured RPC for that chain is blocked.PR addresses the issue by adding a separate lock to sync operations related to ChainInfo separately from subscription management.