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.

Sign up for GitHub

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

Jump to bottom

fix: wg.Done() must be outside RecoverRepanic #247

Merged
merged 2 commits into from
Jun 15, 2020
Merged

fix: wg.Done() must be outside RecoverRepanic #247

merged 2 commits into from
Jun 15, 2020

Commits on Jun 9, 2020

  1. fix: wg.Done() must be outside RecoverRepanic 

    This is a small change to the example for correctness.
The concurrency issue is subtle, but here is an attempt to explain it.

Keep in mind that:
- deferred calls happen in reverse order.
- to recover from panics in f, RecoverRepanic must defer a func call
before calling f.

If defer wg.Done() is part of f, the argument passed to RecoverRepanic,
it means it will be called before any deferred functions setup by
RecoverRepanic itself. If all goroutines started in the loop would call
wg.Done() before an event is reported to Sentry, the main goroutine
blocked on wg.Wait() would unblock and reach the end of func main,
terminating the execution of the program, consequently dropping any
other pending work in other goroutines, reporting to Sentry included.
    @rhcarvalho
    rhcarvalho committed Jun 9, 2020
    Configuration menu
    Copy the full SHA
    2c85fd3 View commit details
    Browse the repository at this point in the history

  2. docs: Reinforce that RecoverRepanic may report only one panic 

    Even though already documented in the body of the function, promoting a
note right at the documentation of RecoverRepanic will hopefully raise
awareness for anyone trying to use the example.
    @rhcarvalho
    rhcarvalho committed Jun 9, 2020
    Configuration menu
    Copy the full SHA
    d6ed47a View commit details
    Browse the repository at this point in the history