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

Race condition in client.receive() goroutine #18

Closed
bodqhrohro opened this issue Dec 18, 2019 · 0 comments
Closed

Race condition in client.receive() goroutine #18

bodqhrohro opened this issue Dec 18, 2019 · 0 comments

Comments

@bodqhrohro
Copy link

Sometimes the client.Close() API call does not complete in time and returns the "response catching timeout". As a fallback, I just close the listener when the timeout has happened, so new updates won't be handled. However, the receive() goroutine does not gracefully handle this, even though it's supposed to, and occasionally panics:

panic: send on closed channel

goroutine 15 [running]:
github.com/zelenin/go-tdlib/client.(*Client).receive(0xc00007ea00)
	/opt/gone/pkg/mod/github.com/zelenin/go-tdlib@v0.1.0/client/client.go:98 +0x19a
created by github.com/zelenin/go-tdlib/client.NewClient
	/opt/gone/pkg/mod/github.com/zelenin/go-tdlib@v0.1.0/client/client.go:69 +0x198

The code that causes the panic is:

                for _, listener := range client.listenerStore.Listeners() {
                        if listener.IsActive() {
                                listener.Updates <- typ
                        } else {
                                needGc = true
                        }
                }

The IsActive() and Close() calls themselves are protected with locks, so the isActive flag theoretically shouldn't be set when the listener.Updates channel is closed already. But it's still possible that the Close() call happens between the IsActive() check and the channel write operation. So a more robust mechanism is needed.

I propose a following pattern:

func (listener *Listener) Transaction(callback func()) bool {
    listener.mu.Lock()
    defer listener.mu.Unlock()

    if listener.isActive {
        callback()
        return true
    }
    return false
}
if !listener.Transaction(func() {
    listener.Updates <- typ
}) {
    needsGc = true
}

This way, the channel is guaranteed to remain open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants