-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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 #6416 #6612
fix #6416 #6612
Conversation
@@ -248,94 +248,93 @@ public async Task<TrialResult> RunAsync(CancellationToken ct = default) | |||
var parameter = tuner.Propose(trialSettings); | |||
trialSettings.Parameter = parameter; | |||
|
|||
using (var trialCancellationTokenSource = new CancellationTokenSource()) | |||
var trialCancellationTokenSource = new CancellationTokenSource(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only actual change: removing using
statement and leave disposing of trialCancellationTokenSource to GC
/azp run |
Azure Pipelines successfully started running 2 pipeline(s). |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #6612 +/- ##
==========================================
- Coverage 68.39% 68.37% -0.02%
==========================================
Files 1176 1176
Lines 248210 248210
Branches 25915 25915
==========================================
- Hits 169759 169724 -35
- Misses 71676 71711 +35
Partials 6775 6775
Flags with carried forward coverage won't be shown. Click here to find out more. |
We are excited to review your PR.
So we can do the best job, please check:
Fixes #nnnn
in your description to cause GitHub to automatically close the issue(s) when your PR is merged.The cause of #6416 is simple: calling
Cancel
on a disposedCancellationTokenSource
. TheCancellationTokenSource
might be disposed when the handler method get called.There're three ways to fix this issue
CancellationTokenSource
get disposed before cancelingCancellationTokenSource
and leave it to GCEventually I choose the third way because it's challenging to dispose object correctly in multi-thread situation. And checking if an object is diposed before calling seems to verbose.