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

Avoid accessing a disposed CancellationSeries #53936

Merged
merged 3 commits into from
Jun 10, 2021

Conversation

sharwell
Copy link
Member

@sharwell sharwell commented Jun 8, 2021

Fixes an error observed in integration test logs:

06/08/2021 01:28:51 Coordinated Universal Time: Error : 1 :[devenv:7680] Unexpected exception: System.AggregateException: One or more errors occurred. ---> System.ObjectDisposedException: Cannot access a disposed object.
Object name: 'CancellationSeries'.
   at Roslyn.Utilities.CancellationSeries.CreateNext(CancellationToken token) in /_/src/Workspaces/Core/Portable/Utilities/CancellationSeries.cs:line 105
   at Microsoft.CodeAnalysis.Editor.Implementation.Classification.CompilationAvailableTaggerEventSource.OnEventSourceChanged(Object sender, TaggerEventArgs args) in /_/src/EditorFeatures/Core/Implementation/Classification/CompilationAvailableTaggerEventSource.cs:line 77
   at Microsoft.CodeAnalysis.Editor.Shared.Tagging.AbstractTaggerEventSource.RaiseChanged() in /_/src/EditorFeatures/Core/Shared/Tagging/EventSources/AbstractTaggerEventSource.cs:line 22
   at Microsoft.CodeAnalysis.Editor.Shared.Tagging.TaggerEventSources.WorkspaceChangedEventSource.<.ctor>b__1_0(CancellationToken cancellationToken) in /_/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.WorkspaceChangedEventSource.cs:line 31
   at Roslyn.Utilities.AsyncBatchingDelay.OnNotifyAsync(ImmutableArray`1 _, CancellationToken cancellationToken) in /_/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingDelay.cs:line 41
   at Roslyn.Utilities.AsyncBatchingWorkQueue`1.ProcessNextBatchAsync(CancellationToken cancellationToken) in /_/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue.cs:line 190
   at Roslyn.Utilities.AsyncBatchingWorkQueue`1.<TryKickOffNextBatchTask>b__16_1(Task _) in /_/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue.cs:line 166
   at Roslyn.Utilities.TaskExtensions.<>c__DisplayClass26_1.<ContinueWithAfterDelayFromAsync>b__1(Task`1 _) in /_/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/TaskExtensions.cs:line 502
   at System.Threading.Tasks.ContinuationResultTaskFromResultTask`2.InnerInvoke()
   at System.Threading.Tasks.Task.Execute()
   --- End of inner exception stack trace ---
   at Microsoft.CodeAnalysis.ErrorReporting.WatsonExtensions.SetCallstackIfEmpty(Exception exception) in /_/src/VisualStudio/Core/Def/Implementation/Watson/WatsonExtensions.cs:line 110
---> (Inner Exception #0) System.ObjectDisposedException: Cannot access a disposed object.
Object name: 'CancellationSeries'.
   at Roslyn.Utilities.CancellationSeries.CreateNext(CancellationToken token) in /_/src/Workspaces/Core/Portable/Utilities/CancellationSeries.cs:line 105
   at Microsoft.CodeAnalysis.Editor.Implementation.Classification.CompilationAvailableTaggerEventSource.OnEventSourceChanged(Object sender, TaggerEventArgs args) in /_/src/EditorFeatures/Core/Implementation/Classification/CompilationAvailableTaggerEventSource.cs:line 77
   at Microsoft.CodeAnalysis.Editor.Shared.Tagging.AbstractTaggerEventSource.RaiseChanged() in /_/src/EditorFeatures/Core/Shared/Tagging/EventSources/AbstractTaggerEventSource.cs:line 22
   at Microsoft.CodeAnalysis.Editor.Shared.Tagging.TaggerEventSources.WorkspaceChangedEventSource.<.ctor>b__1_0(CancellationToken cancellationToken) in /_/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.WorkspaceChangedEventSource.cs:line 31
   at Roslyn.Utilities.AsyncBatchingDelay.OnNotifyAsync(ImmutableArray`1 _, CancellationToken cancellationToken) in /_/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingDelay.cs:line 41
   at Roslyn.Utilities.AsyncBatchingWorkQueue`1.ProcessNextBatchAsync(CancellationToken cancellationToken) in /_/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue.cs:line 190
   at Roslyn.Utilities.AsyncBatchingWorkQueue`1.<TryKickOffNextBatchTask>b__16_1(Task _) in /_/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue.cs:line 166
   at Roslyn.Utilities.TaskExtensions.<>c__DisplayClass26_1.<ContinueWithAfterDelayFromAsync>b__1(Task`1 _) in /_/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/TaskExtensions.cs:line 502
   at System.Threading.Tasks.ContinuationResultTaskFromResultTask`2.InnerInvoke()
   at System.Threading.Tasks.Task.Execute()<---

@sharwell sharwell requested a review from a team as a code owner June 8, 2021 02:19
A copy of the CancellationToken is kept, which is safe to access after
the source is cancelled and disposed.
@@ -69,7 +70,7 @@ private sealed partial class TagSource : ForegroundThreadAffinitizedObject
/// Series of tokens used to cancel previous outstanding work when new work comes in. Also used as the lock
/// to ensure threadsafe writing of _eventWorkQueue.
/// </summary>
private readonly CancellationSeries _cancellationSeries;
private readonly ReferenceCountedDisposable<CancellationSeries> _cancellationSeries;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it very much feels like it takse a bunch of extra tuff just to properly handle cancellation lifetime :(

Copy link
Member Author

@sharwell sharwell Jun 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the object is both disposable and asynchronous, yes.

The problem was always known. The difference here is the problem and fix are visible.

@sharwell sharwell merged commit a995484 into dotnet:release/dev16.11 Jun 10, 2021
@sharwell sharwell deleted the fix-dispose branch June 10, 2021 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants