Close errnotifier chan on err #2102
Conversation
rht
force-pushed
the
fix/logwriter-stall
branch
3 times, most recently
from
b3094ab to
d72ffae
Compare
great 👍
If there's a concrete way out of that test failure that's in progress, that's okay -- I wouldn't want to just work around failing tests though. |
ghost
added
the
|
The stall could be a golang gc bug on osx. This could be tested by running the latest master on an earlier version of golang.
Indeed, already in 0.4.0, 7341486. Though it could be generalized for operations other than gc and pinning. |
|
@rht what is the status here? |
|
@whyrusleeping this is ready to merge, the only RFCR needed is from yours. |
License: MIT Signed-off-by: rht <[email protected]>
To be increased later when auto-gc & pinning race condition are fixed License: MIT Signed-off-by: rht <[email protected]>
whyrusleeping
force-pushed
the
fix/logwriter-stall
branch
from
d72ffae to
10581b6
Compare
| @@ -41,6 +42,7 @@ func (w *writeErrNotifier) Close() error { | |||
| case w.errs <- io.EOF: | |||
| default: | |||
| } | |||
| close(w.errs) | |||
There was a problem hiding this comment.
i'm concerned this might get called twice and panic, want to maybe nil it out after closing, and do the other appropriate checks on the nil value?
|
@rht I had one comment here, otherwise its ready to go (after a rebase though) |
|
@rht could you take a look at this? |
If the pinning still stalls, then the gc lock 7341486 hasn't fixed the problem. This needs to be tested. This PR's fix to stalling is orthogonal to that pin stall, and is not a workaround. |
This 1. fixes the osx test build stalls, 2. temporarily reduce the count of auto gc test (until #2002 (comment) is handled).
Statistically tested in #2101.