-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
auth: get rid of deadlocking channel passing scheme in simpleTokenTTL #7492
Conversation
LGTM. Is it easy to get a test covering the bad case? /cc @davissp14 do you mind giving this a quick test? |
@xiang90 Sadly I am out tonight, but will be able to test tomorrow morning. |
b5bfd98
to
3797293
Compare
lgtm |
@heyitsanthony thanks a lot for the fix! The changes look good to me. But I could reproduce the test failure on my local env, too:
I'll take a look at the test cases. |
3797293
to
4409932
Compare
Tested out the changes and wasn't able to reproduce the deadlock. Nice work! |
Just use the mutex instead. Fixes etcd-io#7471
@mitake the crash was caused by calling UserDelete when auth is disabled; the ttl keeper was nil so trying to acquire the lock in invalidateUser would crash it. |
@heyitsanthony I see. lgtm after CIs can be passed, thanks! |
Codecov Report
@@ Coverage Diff @@
## master #7492 +/- ##
==========================================
- Coverage 70.73% 70.65% -0.09%
==========================================
Files 245 245
Lines 21486 21487 +1
==========================================
- Hits 15198 15181 -17
- Misses 5156 5176 +20
+ Partials 1132 1130 -2
Continue to review full report at Codecov.
|
Will this be added to 3.1.4? |
@davissp14 yes, it's tagged for backporting |
Just use the mutex instead.
Fixes #7471