Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Panic when calling consContext.Stop() #1453

Closed
evanofslack opened this issue Nov 1, 2023 · 1 comment
Closed

Panic when calling consContext.Stop() #1453

evanofslack opened this issue Nov 1, 2023 · 1 comment
Labels
defect Suspected defect such as a bug or regression

Comments

@evanofslack
Copy link
Contributor

Observed behavior

When making concurrent calls to consContext.Stop(), there exists a race condition that can lead to a panic due to closing a closed chan.

panic: close of closed channel

goroutine 10708 [running]:
github.com/nats-io/nats.go/jetstream.(*pullSubscription).Stop(0x140000a6b00)
        /Users/eslack/go/pkg/mod/github.com/nats-io/nats.go@v1.30.2/jetstream/pull.go:596 +0x40
main.raceToStopConsumer.func2()
        /Users/eslack/Desktop/personal/projects/nats-consumer-nil/main.go:62 +0x30
created by main.raceToStopConsumer
        /Users/eslack/Desktop/personal/projects/nats-consumer-nil/main.go:61 +0x170
exit status 2

This appears to happen due to improper guarding of the close call in jetstream/pull.go.

if atomic.LoadUint32(&s.closed) == 1 {
    return
}
atomic.StoreUint32(&s.closed, 1)
close(s.done)

It appears there is a potential race between the load and the store operations. This should likely be an atomic.CompareAndSwapUint32 instead.

Expected behavior

The expectation is that consContext.Stop() never panics

Server and client version

nats-server v2.9.15
client: 1.31.0

Host environment

No response

Steps to reproduce

Please see this gist that reproduces the issue.
https://gist.github.com/evanofslack/58ca4786dd37043efe4497e9d665c6d0

@evanofslack evanofslack added the defect Suspected defect such as a bug or regression label Nov 1, 2023
@piotrpio
Copy link
Collaborator

piotrpio commented Dec 1, 2023

Thank you for the PR. It's merged so I'll be closing the issue, fix will be included in the next release.

@piotrpio piotrpio closed this as completed Dec 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defect Suspected defect such as a bug or regression
Projects
None yet
Development

No branches or pull requests

2 participants