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

Fix race condition in closing a relay #189

Merged
merged 1 commit into from May 7, 2021
Merged

Conversation

9seconds
Copy link
Owner

@9seconds 9seconds commented May 6, 2021

This fixes #188

This commit fixes a situration when relay can be reset before all
waiting goroutines are finished. For example, we terminate processing
based on some event: socket error etc. So, error happens and context is
cancelled. After that a main relay goroutine starts to wait. Meanwhile a
second goroutine reaches deferred function and set wg to done. It means
that main goroutine can continue.

In this case this is really possible that we can start resetting before
transmit goroutine really exits.

A correct solution is to always do wg.Done() as a first deferred thing
on entering to a function. In that case we do not need reordering and so
on.

@9seconds 9seconds mentioned this pull request May 6, 2021
This commit fixes a situration when relay can be reset before all
waiting goroutines are finished. For example, we terminate processing
based on some event: socket error etc. So, error happens and context is
cancelled. After that a main relay goroutine starts to wait. Meanwhile a
second goroutine reaches deferred function and set wg to done. It means
that main goroutine can continue.

In this case this is really possible that we can start resetting before
transmit goroutine really exits.

A correct solution is to always do wg.Done() as a first deferred thing
on entering to a function. In that case we do not need reordering and so
on.
@9seconds 9seconds force-pushed the relay-close-racecondition branch from af4bcc6 to 16f9ec6 Compare May 6, 2021 19:08
@9seconds 9seconds merged commit b19f491 into master May 7, 2021
@9seconds 9seconds deleted the relay-close-racecondition branch May 7, 2021 08:40
@9seconds 9seconds mentioned this pull request May 17, 2021
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

Successfully merging this pull request may close these issues.

Random crashes
1 participant