Skip to content

Conversation

ilonatommy
Copy link
Member

@ilonatommy ilonatommy commented Jul 30, 2025

Prevent scope-related operations on disposed circuit

 ---> System.ObjectDisposedException: Cannot access a disposed object.
Object name: 'IServiceProvider'.
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.ThrowHelper.ThrowObjectDisposedException()
 at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetRequiredService[T](IServiceProvider provider)
   at Microsoft.AspNetCore.Components.Server.Circuits.CircuitPersistenceManager.PauseCircuitAsync(CircuitHost circuit, Boolean saveStateToClient, CancellationToken cancellation) in /home/vsts/work/1/s/src/Components/Server/src/Circuits/CircuitPersistenceManager.cs:line 25

Description

The problem occurs only when we try to pause the circuit after it was already disposed. It can happen in scenarios:

  • Race between eviction and pausing.
  • Race between termination and pausing.
  • Race between two pauses.

The test call these actions synchronously (they await), so that the reproduction is stable. The real race was observed on CI with a frequency of ~20 hits per week.

Fix

  • All disposal actions happen on a single-threaded dispatcher:
    await Renderer.Dispatcher.InvokeAsync(async () =>

    Wrapping pause logic in the dispatcher invoke prevents it from running concurrently with disposal that comes from e.g. termination or eviction.
    - Even if pause will not run concurrently with disposal, it can still run just after disposal happened - see the tests. To prevent it from trying to discover services in a disposed DI scope, we check CircuitHost disposal flag. removed after the discussion - I could not find any evidence that pause can happen after disposal in real life.

Fixes - let's keep it open for some time: #62672

Frequency of failures on CI before this fix for reference:
image

@ilonatommy ilonatommy added this to the 10.0-rc1 milestone Jul 30, 2025
@ilonatommy ilonatommy self-assigned this Jul 30, 2025
@ilonatommy ilonatommy added the area-blazor Includes: Blazor, Razor Components label Jul 30, 2025
@ilonatommy ilonatommy marked this pull request as ready for review July 31, 2025 08:54
@Copilot Copilot AI review requested due to automatic review settings July 31, 2025 08:54
@ilonatommy ilonatommy requested a review from a team as a code owner July 31, 2025 08:54
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes race conditions that occur when attempting to pause a circuit after it has already been disposed, preventing exceptions from being thrown when resolving services from disposed DI scopes.

  • Wraps pause logic in the circuit's single-threaded dispatcher to prevent concurrent execution with disposal operations
  • Adds a disposal check before attempting to resolve services during pause operations
  • Includes comprehensive test coverage for various race condition scenarios

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/Components/Server/src/Circuits/CircuitPersistenceManager.cs Wraps pause logic in dispatcher invoke and adds disposal check
src/Components/Server/src/Circuits/CircuitHost.cs Adds internal method to expose disposal state
src/Components/Server/src/Circuits/CircuitRegistry.cs Changes method visibility from private to internal for testing
src/Components/Server/test/Circuits/CircuitRegistryTest.cs Adds comprehensive test coverage for race condition scenarios

@ilonatommy
Copy link
Member Author

e2e on 3rd run passed, re-merging

@ilonatommy ilonatommy changed the title Avoid pausing circuit when it's disposed Avoid pausing circuit simultaneously with disposing of it Aug 5, 2025
@ilonatommy ilonatommy requested a review from javiercn August 5, 2025 07:35
@ilonatommy ilonatommy enabled auto-merge (squash) August 5, 2025 09:31
@ilonatommy ilonatommy merged commit a45c4ad into dotnet:main Aug 5, 2025
28 of 30 checks passed
@pavelsavara
Copy link
Member

nice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-blazor Includes: Blazor, Razor Components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants