Skip to content

Commit 572e586

Browse files
Copilotanacrolix
andauthored
Backport TestResubscribeWithCompletedSubscription and deadlock fix to release/3.3 (#18862)
Backports test case and deadlock fix from PR #18562 (commit `f3e42e7626`) that validates `Unsubscribe()` doesn't deadlock when the subscription producer exits before unsubscription. ## Changes - **Test case**: `TestResubscribeWithCompletedSubscription` in `p2p/event/subscription_test.go` - Validates no deadlock when producer completes before `Unsubscribe()` is called - **Deadlock fix**: Make `resubscribeSub.unsub` channel buffered (size 1) - Without buffer, `Unsubscribe()` blocks when sending after resubscribe loop exits - Buffer allows send to complete even when no receiver is present ```go // Before: unbuffered channel causes deadlock unsub: make(chan struct{}) // After: buffered channel prevents deadlock unsub: make(chan struct{}, 1) ``` The test reproduces the deadlock scenario by ensuring the subscription completes successfully before calling `Unsubscribe()`. ## Original Source - **Original PR**: #18562 - "Sync p2p/event with geth's event" - **Original Commit**: f3e42e7 <!-- START COPILOT ORIGINAL PROMPT --> <details> <summary>Original prompt</summary> > Backport the test case `TestResubscribeWithCompletedSubscription` from the `main` branch of Erigon to the branch `release/3.3`. This test validates that the `Unsubscribe` method does not result in a deadlock when the producer exits first. Reference implementation in `p2p/event/subscription_test.go` from commit `b8d1242103d81d46591cf86c71d8e182bc8c2add`. Ensure it only modifies the `release/3.3` branch and includes the additional test case. </details> <!-- START COPILOT CODING AGENT SUFFIX --> *This pull request was created from Copilot chat.* > <!-- START COPILOT CODING AGENT TIPS --> --- ✨ Let Copilot coding agent [set things up for you](https://github.com/erigontech/erigon/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: anacrolix <988750+anacrolix@users.noreply.github.com>
1 parent d176276 commit 572e586

File tree

2 files changed

+25
-1
lines changed

2 files changed

+25
-1
lines changed

p2p/event/subscription.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ func ResubscribeErr(backoffMax time.Duration, fn ResubscribeErrFunc) Subscriptio
123123
backoffMax: backoffMax,
124124
fn: fn,
125125
err: make(chan error),
126-
unsub: make(chan struct{}),
126+
unsub: make(chan struct{}, 1),
127127
}
128128
go s.loop()
129129
return s

p2p/event/subscription_test.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,3 +156,27 @@ func TestResubscribeWithErrorHandler(t *testing.T) {
156156
t.Fatalf("unexpected subscription errors %v, want %v", subErrs, expectedSubErrs)
157157
}
158158
}
159+
160+
func TestResubscribeWithCompletedSubscription(t *testing.T) {
161+
t.Parallel()
162+
163+
quitProducerAck := make(chan struct{})
164+
quitProducer := make(chan struct{})
165+
166+
sub := ResubscribeErr(100*time.Millisecond, func(ctx context.Context, lastErr error) (Subscription, error) {
167+
return NewSubscription(func(unsubscribed <-chan struct{}) error {
168+
select {
169+
case <-quitProducer:
170+
quitProducerAck <- struct{}{}
171+
return nil
172+
case <-unsubscribed:
173+
return nil
174+
}
175+
}), nil
176+
})
177+
178+
// Ensure producer has started and exited before Unsubscribe
179+
close(quitProducer)
180+
<-quitProducerAck
181+
sub.Unsubscribe()
182+
}

0 commit comments

Comments
 (0)