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

Microsoft.Extensions.Http.Resilience - Cannot access a disposed object. - Integrationtest with WebApplicationFactory #97037

Closed
jnormen opened this issue Jan 11, 2024 · 29 comments
Assignees
Labels
area-Extensions-Options bug source-generator Indicates an issue with a source generator feature
Milestone

Comments

@jnormen
Copy link

jnormen commented Jan 11, 2024

Problem with .AddStandardResilienceHandler(); when used in integration test with custom web factory and when there is more than one test.

I have a service that uses HttpClients to 3rd party endpoints. On this one, I use the.AddStandardResilienceHandler();
I set this up in my startup.cs then I have an integration test where I use the WebApplicationFactory

All works fine, but after some lib updates, I start getting random exceptions from my tests.
If I run one test at a time with no problem, it seems to be shared resource issues when they run in parallel.

The tests randomly give me:

  Message: 
System.ObjectDisposedException : Cannot access a disposed object.
Object name: 'IServiceProvider'.

And:

System.InvalidCastException: Unable to cast object of type 'System.TimeSpan' to type 'System.String'.
   at __OptionValidationGeneratedAttributes.<Validators_g>F8B1F3D42962A35D8FF1B1489612AEF6C36F3713335EFF79DB68A25973333C495____SourceGen__RangeAttribute.IsValid(Object value)
   at System.ComponentModel.DataAnnotations.ValidationAttribute.IsValid(Object value, ValidationContext validationContext)
   at System.ComponentModel.DataAnnotations.ValidationAttribute.GetValidationResult(Object value, ValidationContext validationContext)
   at System.ComponentModel.DataAnnotations.Validator.TryValidate(Object value, ValidationContext validationContext, ValidationAttribute attribute, ValidationError& validationError)
   at System.ComponentModel.DataAnnotations.Validator.GetValidationErrors(Object value, ValidationContext validationContext, IEnumerable`1 attributes, Boolean breakOnFirstError)
   at System.ComponentModel.DataAnnotations.Validator.TryValidateValue(Object value, ValidationContext validationContext, ICollection`1 validationResults, IEnumerable`1 validationAttributes)
   at Microsoft.Extensions.Http.Resilience.__HttpTimeoutStrategyOptionsValidator__.

It does not happen all the time but mostly every time.

I have tried fixture, using the same client that's created on all test methods but it seems like a disposal is happening.
But next time there is some expectation that the resilience services shall exist somehow. Not all services are not resettled.
Some singleton was added once I guess.

 public HttpClient CreateClient(ITestOutputHelper testOutputHelper = null)
 {
     _waf = new WebApplicationFactory<TProgram>()
        .WithWebHostBuilder(builder =>
        {

If I remove the .AddStandardResilienceHandler(); from my setup of the HTTP client all works fine.

I also tried to remove the added services and override them in

 builder.ConfigureTestServices(services =>
 {

But that will not work because it's still have added the resilience stuff before I reach this area.

So it seams like:
First it creates my client with resilience, then I ran the fist FACT test. Then a second one.
I have 2 tests one for success and one for failure. If I remove one of the test all works fine. But as soon I share the client in many test this happens.

Not sure if there is som clean-up, or dispose issues with the resilience setup? I have followed the basic guidelines to setup integration tests in .net

Have I missed something? Or is this a known bug?
I can see there is a bug reported where it seams to be a shared setting for all services atm so we can't override settings on different once. Maybe that's related here?

@jnormen jnormen added the untriaged New issue has not been triaged by the area owner label Jan 11, 2024
@jnormen jnormen changed the title Microsoft.Extensions.Http.Resilience Microsoft.Extensions.Http.Resilience - Cannot access a disposed object. - Integrationtest with WebApplicationFactory Jan 11, 2024
@geeknoid geeknoid added bug and removed untriaged New issue has not been triaged by the area owner labels Jan 11, 2024
@geeknoid
Copy link
Member

Thanks for your report. I think we might be dealing with multiple different issues, given the different symptoms.

Regarding the stack trace that ends with an InvalidCastException, that one is very puzzling. I just ran a test that exercises this code, and it never gets to a point where it is casting a TimeSpan into a string. I tried both a valid timespan and an invalid timespan to make sure I try out both success and failure validation paths. This code path should not be subject to non-deterministic behavior, either it works or it doesn't work. So I don't know what's going on here.

Are you running the .NET 8 runtime with .NET 8 versions of all extensions? The only explanation I have is that you're somehow using an older version of the RangeAttribute type which didn't support the semantics this code is depending on.

@jnormen
Copy link
Author

jnormen commented Jan 11, 2024

I set it up with Refit so maybe related to that?
This starts to happen after 3 months without an issue. I have updated some .net libs so not sure exactly when it started.

I use .net 8 final version. The tests run with xUnit latest version as well.

The strange thing is that it feels like something is not disposing but some parts is. Then it try to get something from the IServiceCollection that is disposed, like the standard settings the recilienece library seams to set up. The Options of default values? And in that case if IServcieCollection somehow gets disposed but some other stuffs are not it try to inject the default settings and get wrong values that are not valid?

Its so hard to debug because it happens when two test or more run in within the same class. So I can never get this problem when run each test one by one manually.

But I do not get why IServiceCollection is disposed any way, the exception start when it try to create and use the client.

If I remove the .AddStandardResilienceHandler(); from the code the error gets away. It would be easier If I could get this to happen all the time :)

I'm using
System.Net.Http" Version="4.3.4"
"FluentAssertions" Version="6.12.0"
"FluentValidation" Version="11.9.0"
"Microsoft.NET.Test.Sdk" Version="17.8.0"
"Microsoft.VisualStudio.Threading.Analyzers" Version="17.8.14"
"SonarAnalyzer.CSharp" Version="9.16.0.82469"
"xunit" Version="2.6.5"
"xunit.extensibility.core" Version="2.6.5"
xunit.runner.visualstudio" Version="2.5.6
"Refit" Version="7.0.0"
"Refit.HttpClientFactory" Version="7.0.0"

.net 8
With languange set to 12 in main project and my test projects

I have not test it without Refit and had no time test it more so I just added a if statement so it does not add that line for my tests :( I shall see if I have more time to do even more tests. After 1 day testing with different disposing, order of registrations, change my test to use fixture, no fixture etc without any progress I searched here and saw the issue about shared settings and thought maybe this can be related and a know issue.

@geeknoid
Copy link
Member

It doesn't feel as though Refit is to blame here. There could definitely be a race here due to some global state being attached somewhere. I can see where the multiple disposal issue could surface in that context.

But the InvalidCastException appears highly mysterious. If there's any way you could catch it in the debugger when this happens and look around to see how we got there, maybe we can figure out what's happening in that particular case. And who knows, maybe that will shed some light on the multiple disposal issue as well.

@jnormen
Copy link
Author

jnormen commented Jan 12, 2024

Sadly I have limited skills here in deep debugging when it comes to catching it from a random situation.

All I can get is more info about the whole error thought, not sure if it gives more info.

Message: 
System.ObjectDisposedException : Cannot access a disposed object.
Object name: 'IServiceProvider'.

  Stack Trace: 
ThrowHelper.ThrowObjectDisposedException()
ServiceProvider.GetService(ServiceIdentifier serviceIdentifier, ServiceProviderEngineScope serviceProviderEngineScope)
ServiceProvider.GetService(Type serviceType)
ServiceProviderServiceExtensions.GetRequiredService(IServiceProvider provider, Type serviceType)
ServiceProviderServiceExtensions.GetRequiredService[T](IServiceProvider provider)
WebApplicationFactory`1.ConfigureHostBuilder(IHostBuilder hostBuilder)
WebApplicationFactory`1.EnsureServer()
WebApplicationFactory`1.CreateDefaultClient(DelegatingHandler[] handlers)
WebApplicationFactory`1.CreateDefaultClient(Uri baseAddress, DelegatingHandler[] handlers)
WebApplicationFactory`1.CreateClient(WebApplicationFactoryClientOptions options)
WebApplicationFactory`1.CreateClient()
CustomWebApplicationFactory`1.CreateClient(ITestOutputHelper testOutputHelper) line 66
AddUpdateStoreTest.StoreImportRequest_Should_Be_Valid() line 54
AddUpdateStoreTest.StoreImportRequest_Should_Be_Valid() line 61
--- End of stack trace from previous location ---

  Standard Output: 
Hosting failed to start
System.InvalidCastException: Unable to cast object of type 'System.TimeSpan' to type 'System.String'.
   at __OptionValidationGeneratedAttributes.<Validators_g>F8B1F3D42962A35D8FF1B1489612AEF6C36F3713335EFF79DB68A25973333C495____SourceGen__RangeAttribute.IsValid(Object value)
   at System.ComponentModel.DataAnnotations.ValidationAttribute.IsValid(Object value, ValidationContext validationContext)
   at System.ComponentModel.DataAnnotations.ValidationAttribute.GetValidationResult(Object value, ValidationContext validationContext)
   at System.ComponentModel.DataAnnotations.Validator.TryValidate(Object value, ValidationContext validationContext, ValidationAttribute attribute, ValidationError& validationError)
   at System.ComponentModel.DataAnnotations.Validator.GetValidationErrors(Object value, ValidationContext validationContext, IEnumerable`1 attributes, Boolean breakOnFirstError)
   at System.ComponentModel.DataAnnotations.Validator.TryValidateValue(Object value, ValidationContext validationContext, ICollection`1 validationResults, IEnumerable`1 validationAttributes)
   at Microsoft.Extensions.Http.Resilience.__HttpRetryStrategyOptionsValidator__.Validate(String name, HttpRetryStrategyOptions options)
   at Microsoft.Extensions.Http.Resilience.Internal.Validators.HttpStandardResilienceOptionsValidator.Validate(String name, HttpStandardResilienceOptions options)
   at Microsoft.Extensions.Options.OptionsFactory`1.Create(String name)
   at Microsoft.Extensions.Options.OptionsMonitor`1.<>c.<Get>b__10_0(String name, IOptionsFactory`1 factory)
   at Microsoft.Extensions.Options.OptionsCache`1.<>c__DisplayClass3_1`1.<GetOrAdd>b__2()
   at System.Lazy`1.ViaFactory(LazyThreadSafetyMode mode)
   at System.Lazy`1.ExecutionAndPublication(LazyHelper executionAndPublication, Boolean useDefaultConstructor)
   at System.Lazy`1.CreateValue()
   at Microsoft.Extensions.Options.OptionsCache`1.GetOrAdd[TArg](String name, Func`3 createOptions, TArg factoryArgument)
   at Microsoft.Extensions.Options.OptionsMonitor`1.Get(String name)
   at Microsoft.Extensions.DependencyInjection.OptionsBuilderExtensions.<>c__DisplayClass0_1`1.<ValidateOnStart>b__1()
   at Microsoft.Extensions.Options.StartupValidator.Validate()
   at Microsoft.Extensions.Hosting.Internal.Host.StartAsync(CancellationToken cancellationToken)

And this is the code that set it up in startup.cs

 builder.Services.AddRefitClient<TApiInterface>()
     .ConfigureHttpClient(c =>
     {
         c.BaseAddress = new Uri(configuration.GetValue<string>(endpointConfigKey)!);
         c.Timeout = TimeSpan.FromSeconds(35);
     })
     .AddHttpMessageHandler<AuthHeaderHandler>()
     .AddStandardResilienceHandler();

And my factory I created that I used with my Fixture setup or I have also tried to just use it in a constructor of my unit test.
I also tried to call the create in each test and set the test to not run in parallel, just for testing. Same results on all 3 different ways.

internal class CustomWebApplicationFactory<TProgram> : IAsyncDisposable where TProgram : class
{
    private readonly Action<IServiceCollection> _serviceOverride;
    private WebApplicationFactory<TProgram> _waf;

    public CustomWebApplicationFactory(Action<IServiceCollection> serviceOverride = null)
    {
        _serviceOverride = serviceOverride;
    }

    public HttpClient CreateClient(ITestOutputHelper testOutputHelper = null)
    {
        _waf = new WebApplicationFactory<TProgram>()
           .WithWebHostBuilder(builder =>
           {
               builder.UseEnvironment("IntegrationTesting"); //// This will make it possible for us to have a appsettings.IntegrationTesting.Json file

               builder.ConfigureLogging(x =>
               {
                   x.ClearProviders();
                   if (testOutputHelper != null)
                       x.Services.AddSingleton<ILoggerProvider>(new XUnitLoggerProvider(testOutputHelper));
               });

               // Add mock/test services to the builder here
               builder.ConfigureTestServices(services =>
               {
                   services.AddOptions<ExternalApiConfig>().Configure(options =>
                   {
                       options.Username = "foo";
                       options.Password = "bar";
                   });

                   services.RemoveAll<IProdApi>();
                   services.RemoveAll<ITestApi>();

                   _serviceOverride?.Invoke(services);
               });
           });
        return _waf.CreateClient();
    }

    public ValueTask DisposeAsync()
    {
        return _waf.DisposeAsync();
    }
}

I tried it without the IAsyncDisposable and just IDispose as well.

I'm reusing this factory for different tests that exist in different classes. I have this action so I can easy
setup mocks etc... if needed. Thats why the _serviceOverride?.Invoke(services);
I have not tried without that one, but I have tried without send in any action when creating the client.

@geeknoid
Copy link
Member

Would your code be available publicly somewhere? I'd love to get it and try this locally.

Thanks.

@martintmk
Copy link

martintmk commented Jan 12, 2024

I have a strong suspicion that this is not related to resilience code, but rather something around options or options validation.

To get resilience out of the picture try to do the following:

var builder = builder.Services.AddRefitClient<TApiInterface>()
    .ConfigureHttpClient(c =>
    {
        c.BaseAddress = new Uri(configuration.GetValue<string>(endpointConfigKey)!);
        c.Timeout = TimeSpan.FromSeconds(35);
    })
    .AddHttpMessageHandler<AuthHeaderHandler>();

builder.AddStandardResilienceHandler();

// This is the key to named options used inside AddStandardResilienceHandler
var optionsName = builder.Name + "-standard";

// Now resolve the options same way the resilience is doing it
var options = services.BuildServiceProvider()
   .GetRequiredService<IOptionsMonitor<HttpStandardResilienceOptions>>()
   .Get(optionsName);

I suspect that the call to retrieve the options will fail.

@jnormen
Copy link
Author

jnormen commented Jan 14, 2024

Would your code be available publicly somewhere? I'd love to get it and try this locally.

Thanks.

Sadly I do not :/ the code is located in a private repository at a company, so it's not public :( I need to talk with them to see what I can do public or not in that case

@jnormen
Copy link
Author

jnormen commented Jan 14, 2024

I have a strong suspicion that this is not related to resilience code, but rather something around options or options validation.

To get resilience out of the picture try to do the following:

var builder = builder.Services.AddRefitClient<TApiInterface>()
    .ConfigureHttpClient(c =>
    {
        c.BaseAddress = new Uri(configuration.GetValue<string>(endpointConfigKey)!);
        c.Timeout = TimeSpan.FromSeconds(35);
    })
    .AddHttpMessageHandler<AuthHeaderHandler>();

builder.AddStandardResilienceHandler();

// This is the key to named options used inside AddStandardResilienceHandler
var optionsName = builder.Name + "-standard";

// Now resolve the options same way the resilience is doing it
var options = services.BuildServiceProvider()
   .GetRequiredService<IOptionsMonitor<HttpStandardResilienceOptions>>()
   .Get(optionsName);

I suspect that the call to retrieve the options will fail.

When I added:
```
// This is the key to named options used inside AddStandardResilienceHandler
var optionsName = builder2.Name + "-standard";

   // Now resolve the options same way the resilience is doing it
   var options = builder.Services.BuildServiceProvider()
      .GetRequiredService<IOptionsMonitor<HttpStandardResilienceOptions>>()
      .Get(optionsName);
I did not get random dispiose error. But the random error now is that client could not be created. 

`System.InvalidOperationException : The entry point exited without ever building an IHost.`

I could not find any exception to this more then it failed to create the builder. 

I will test one last thing that I have not test yet, becuase of the limitation of time. We are rolling out a system atm so lots of things are in focus

@jnormen
Copy link
Author

jnormen commented Jan 14, 2024

Here is what I can share

The fixture:

  public class ClientTestFixture : IDisposable
  {
      readonly CustomWebApplicationFactory<Program> factory;
      public ClientTestFixture()
      {
          factory = new CustomWebApplicationFactory<Program>();
      }

      public HttpClient Client
      {
          get { return factory.CreateClient(); }
      }

      public void Dispose()
      {
          Dispose(true);
          GC.SuppressFinalize(this);
      }

My factory code:

   internal class CustomWebApplicationFactory<TProgram> : IAsyncDisposable where TProgram : class
{
    private readonly Action<IServiceCollection> _serviceOverride;
    private WebApplicationFactory<TProgram> _waf;
    private readonly object _lock = new object();

    public CustomWebApplicationFactory(Action<IServiceCollection> serviceOverride = null)
    {
        _serviceOverride = serviceOverride;
    }
    public HttpClient CreateClient(ITestOutputHelper testOutputHelper = null)
    {
        lock (_lock)
        {
            if (_waf == null)
            {
                _waf = InitializeFactory(testOutputHelper);
            }
            return _waf.CreateClient();
        }
    }
    private WebApplicationFactory<TProgram> InitializeFactory(ITestOutputHelper testOutputHelper)
    {
        _waf = new WebApplicationFactory<TProgram>()
           .WithWebHostBuilder(builder =>
           {
               builder.UseEnvironment("ContractTesting"); //// This will make it possible for us to have a appsettings.ContractTesting.Json file

               builder.ConfigureLogging(x =>
               {
                   x.ClearProviders();
                   if (testOutputHelper != null)
                       x.Services.AddSingleton<ILoggerProvider>(new XUnitLoggerProvider(testOutputHelper));
               });

               // Add mock/test services to the builder here
               builder.ConfigureTestServices(services =>
               {
                   services.AddOptions<Enactor>().Configure(options =>
                   {
                       options.Username = "foo";
                       options.Password = "bar";
                   });

                   services.RemoveAll<IProdEnactorApi>();
                   services.RemoveAll<ITestEnactorApi>();
                  
                   services.RemoveAll(typeof(IMediator));

                   var fakeMediator = A.Fake<IMediator>();
                   services.AddScoped((x) => { return fakeMediator; });

                   _serviceOverride?.Invoke(services);
               });
           });
        return _waf;
    }


    public ValueTask DisposeAsync()
    {
        return _waf.DisposeAsync();
    }
}

The Lock part is my last experiment, so In general I do not use that, but it does not matter, same result any way.

One of 2 test. I have two classes for 2 different feautres. Boot have 2 test with same pattern.

public class AddUpdateStoreTest : IClassFixture<ClientTestFixture>
{
    readonly HttpClient client;
    public AddUpdateStoreTest(ITestOutputHelper testOutputHelper, ClientTestFixture fixture)
    {

        client = fixture.Client;
    }
    public static StoreImportRequest GetStoreImportRequest()
    {
        return new StoreImportRequest(
            StoreNumber: 9902,
            IsActive: true,
            OpenDate: DateOnly.FromDateTime(DateTime.Now),
            Name: "Test Store",
            Country: "SE",
            IsTestStore: true,
            Email: "somesotre@somestore.com",
            PhoneNumber: "+46(0)739525320",
            ZipCode: "44 1123",
            Street: "Test Street",
            City: "Göteborg");
    }

    [Fact]
    public async Task StorePostRequest_Should_Be_Valid()
    {
        // Arrange
        var request = AddUpdateStoreTest.GetStoreImportRequest();

        client.DefaultRequestHeaders.Add("x-environment", "test");

        // Act
        using var response = await client.PostAsJsonAsync("store/location", request);

        // Assert
        response.StatusCode.Should().Be(HttpStatusCode.Accepted);
    }

    [Fact]
    public async Task StorePostRequest_Should_Not_Be_Valid()
    {
        // Arrange
        var request = AddUpdateStoreTest.GetStoreImportRequest();
       
        client.DefaultRequestHeaders.Add("x-environment", "test");

        // "Modify" the record by creating a new copy with a not valid Email
        StoreImportRequest storeImportRequest = request with { Name = string.Empty };

        // Act
        using var response = await client.PostAsJsonAsync("store/location", storeImportRequest);

        // Assert
        response.StatusCode.Should().Be(HttpStatusCode.BadRequest);
    }
}

I'm sorry I cannot share a project for you. If this does not help. I will see if I have time tomorrow to create a whole new project that I can share.

if not, then I turn it off, go back to old polly for a short time period, and see if this is something others will report.

@martintmk
Copy link

@jnormen

This is very strange error:

System.InvalidOperationException : The entry point exited without ever building an IHost.

Probably some host setup issue? I am afraid that at this point I am not able to help further, until you provide some repro code.

@jnormen
Copy link
Author

jnormen commented Jan 15, 2024

@jnormen

This is very strange error:

System.InvalidOperationException : The entry point exited without ever building an IHost.

Probably some host setup issue? I am afraid that at this point I am not able to help further, until you provide some repro code.

I understand that and can see what I can do.

I saw there are some new updates to Polly, resilience lib from 8 to a 8.0.1 or something. I will test that out as soon to see if the problem persists, or might be fixed in those versions.

@jjanuszkiewicz
Copy link

I'm getting the Unable to cast object of type 'System.TimeSpan' to type 'System.String' error too (but not the dispose-related one).

What info would you need to diagnose this? Would a complete solution that shows the problem help?

@martincostello
Copy link
Member

martincostello commented Jan 16, 2024

I've just started noticing this error locally when trying to migrate to the use of ConfigureHttpClientDefaults() from configuring things per-HttpClient. Which tests fail seems to be intermittent - I get failures when I run all the tests, but if I then re-run the individually failing tests in Visual Studio they pass.

The code I'm reproducing this with locally (but not in CI) is here: martincostello/alexa-london-travel@288ef84

Wasn't there a code change in this area since 8.0.0 for an issue to do with native AoT? The stack trace is ringing a bell, but that might just be a coincidence. This is the issue I'm thinking of: #94799, which was marked as fixed for 8.0 by #94857.

Stack trace for a failing test:

System.InvalidCastException: Unable to cast object of type 'System.TimeSpan' to type 'System.String'.
   at __OptionValidationGeneratedAttributes.<Validators_g>F8B1F3D42962A35D8FF1B1489612AEF6C36F3713335EFF79DB68A25973333C495____SourceGen__RangeAttribute.IsValid(Object value)
   at System.ComponentModel.DataAnnotations.ValidationAttribute.IsValid(Object value, ValidationContext validationContext)
   at System.ComponentModel.DataAnnotations.ValidationAttribute.GetValidationResult(Object value, ValidationContext validationContext)
   at System.ComponentModel.DataAnnotations.Validator.TryValidate(Object value, ValidationContext validationContext, ValidationAttribute attribute, ValidationError& validationError)
   at System.ComponentModel.DataAnnotations.Validator.GetValidationErrors(Object value, ValidationContext validationContext, IEnumerable`1 attributes, Boolean breakOnFirstError)
   at System.ComponentModel.DataAnnotations.Validator.TryValidateValue(Object value, ValidationContext validationContext, ICollection`1 validationResults, IEnumerable`1 validationAttributes)
   at Microsoft.Extensions.Http.Resilience.__HttpTimeoutStrategyOptionsValidator__.Validate(String name, HttpTimeoutStrategyOptions options)
   at Microsoft.Extensions.Http.Resilience.Internal.Validators.HttpStandardResilienceOptionsValidator.Validate(String name, HttpStandardResilienceOptions options)
   at Microsoft.Extensions.Options.OptionsFactory`1.Create(String name)
   at System.Lazy`1.ViaFactory(LazyThreadSafetyMode mode)
   at System.Lazy`1.ExecutionAndPublication(LazyHelper executionAndPublication, Boolean useDefaultConstructor)
   at System.Lazy`1.CreateValue()
   at Microsoft.Extensions.Options.OptionsCache`1.GetOrAdd[TArg](String name, Func`3 createOptions, TArg factoryArgument)
   at Microsoft.Extensions.Options.OptionsMonitor`1.Get(String name)
   at Microsoft.Extensions.DependencyInjection.ResilienceHttpClientBuilderExtensions.<>c__DisplayClass15_0.<AddStandardResilienceHandler>b__0(ResiliencePipelineBuilder`1 builder, ResilienceHandlerContext context)
   at Microsoft.Extensions.DependencyInjection.ResilienceHttpClientBuilderExtensions.<>c__DisplayClass8_0.<AddHttpResiliencePipeline>b__0(ResiliencePipelineBuilder`1 builder, AddResiliencePipelineContext`1 context)
   at Polly.PollyServiceCollectionExtensions.<>c__DisplayClass1_1`2.<AddResiliencePipeline>b__2(ResiliencePipelineBuilder`1 builder, ConfigureBuilderContext`1 context)
   at Polly.Registry.RegistryPipelineComponentBuilder`2.CreateBuilder()
   at Polly.Registry.RegistryPipelineComponentBuilder`2.CreateComponent()
   at Polly.Registry.ResiliencePipelineRegistry`1.GenericRegistry`1.<>c__DisplayClass7_0.<GetOrAdd>b__0(TKey k)
   at System.Collections.Concurrent.ConcurrentDictionary`2.GetOrAdd(TKey key, Func`2 valueFactory)
   at Polly.Registry.ResiliencePipelineRegistry`1.GenericRegistry`1.GetOrAdd(TKey key, Action`2 configure)
   at Polly.Registry.ResiliencePipelineRegistry`1.GenericRegistry`1.TryGet(TKey key, ResiliencePipeline`1& strategy)
   at Polly.Registry.ResiliencePipelineRegistry`1.TryGetPipeline[TResult](TKey key, ResiliencePipeline`1& pipeline)
   at Polly.Registry.ResiliencePipelineProvider`1.GetPipeline[TResult](TKey key)
   at Microsoft.Extensions.DependencyInjection.ResilienceHttpClientBuilderExtensions.CreatePipelineSelector(IServiceProvider serviceProvider, String pipelineName)
   at Microsoft.Extensions.DependencyInjection.ResilienceHttpClientBuilderExtensions.<>c__DisplayClass5_0.<AddResilienceHandler>b__0(IServiceProvider serviceProvider)
   at Microsoft.Extensions.DependencyInjection.HttpClientBuilderExtensions.<>c__DisplayClass3_0.<AddHttpMessageHandler>b__1(HttpMessageHandlerBuilder b)
   at Microsoft.Extensions.Http.DefaultHttpClientFactory.<>c__DisplayClass17_0.<CreateHandlerEntry>g__Configure|0(HttpMessageHandlerBuilder b)
   at Microsoft.Extensions.Http.MetricsFactoryHttpMessageHandlerFilter.<>c__DisplayClass2_0.<Configure>b__0(HttpMessageHandlerBuilder builder)
   at Microsoft.Extensions.Http.LoggingHttpMessageHandlerBuilderFilter.<>c__DisplayClass6_0.<Configure>b__0(HttpMessageHandlerBuilder builder)
   at MartinCostello.LondonTravel.Skill.HttpRequestInterceptionFilter.<>c__DisplayClass2_0.<Configure>b__0(HttpMessageHandlerBuilder builder) in C:\Coding\martincostello\alexa-london-travel\test\LondonTravel.Skill.Tests\HttpRequestInterceptionFilter.cs:line 30
   at Microsoft.Extensions.Http.DefaultHttpClientFactory.CreateHandlerEntry(String name)
   at System.Lazy`1.ViaFactory(LazyThreadSafetyMode mode)
   at System.Lazy`1.ExecutionAndPublication(LazyHelper executionAndPublication, Boolean useDefaultConstructor)
   at System.Lazy`1.CreateValue()
   at Microsoft.Extensions.Http.DefaultHttpClientFactory.CreateHandler(String name)
   at Microsoft.Extensions.Http.DefaultHttpClientFactory.CreateClient(String name)
   at Microsoft.Extensions.DependencyInjection.HttpClientBuilderExtensions.AddTransientHelper[TClient](IServiceProvider s, IHttpClientBuilder builder)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSiteMain(ServiceCallSite callSite, TArgument argument)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitDisposeCache(ServiceCallSite transientCallSite, RuntimeResolverContext context)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSite(ServiceCallSite callSite, TArgument argument)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitConstructor(ConstructorCallSite constructorCallSite, RuntimeResolverContext context)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSiteMain(ServiceCallSite callSite, TArgument argument)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitDisposeCache(ServiceCallSite transientCallSite, RuntimeResolverContext context)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSite(ServiceCallSite callSite, TArgument argument)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.Resolve(ServiceCallSite callSite, ServiceProviderEngineScope scope)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.DynamicServiceProviderEngine.<>c__DisplayClass2_0.<RealizeService>b__0(ServiceProviderEngineScope scope)
   at Microsoft.Extensions.DependencyInjection.ServiceProvider.GetService(ServiceIdentifier serviceIdentifier, ServiceProviderEngineScope serviceProviderEngineScope)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.ServiceProviderEngineScope.GetService(Type serviceType)
   at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetRequiredService(IServiceProvider provider, Type serviceType)
   at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetRequiredService[T](IServiceProvider provider)
   at MartinCostello.LondonTravel.Skill.IntentFactory.Create(Intent intent) in C:\Coding\martincostello\alexa-london-travel\src\LondonTravel.Skill\IntentFactory.cs:line 33
   at MartinCostello.LondonTravel.Skill.AlexaSkill.OnIntentAsync(IIntentRequest intent, Session session) in C:\Coding\martincostello\alexa-london-travel\src\LondonTravel.Skill\AlexaSkill.cs:line 71
   at MartinCostello.LondonTravel.Skill.FunctionHandler.HandleRequestAsync(SkillRequest request) in C:\Coding\martincostello\alexa-london-travel\src\LondonTravel.Skill\FunctionHandler.cs:line 58

@martincostello
Copy link
Member

The decompiled code for that method from Microsoft.Extensions.Http.Resilience 8.1.0 is this according to ILSpy is below. Looks like it contains two casts to a string.

____SourceGen__RangeAttribute.IsValid()
public override bool IsValid(object? value)
{
	if (!Initialized)
	{
		if (Minimum == null || Maximum == null)
		{
			throw new InvalidOperationException("The minimum and maximum values must be set to valid values.");
		}
		if (NeedToConvertMinMax)
		{
			CultureInfo formatProvider = (ParseLimitsInInvariantCulture ? CultureInfo.InvariantCulture : CultureInfo.CurrentCulture);
			if (OperandType == typeof(TimeSpan))
			{
				if (!TimeSpan.TryParse((string)Minimum, formatProvider, out var result) || !TimeSpan.TryParse((string)Maximum, formatProvider, out var result2))
				{
					throw new InvalidOperationException("The minimum and maximum values must be set to valid values.");
				}
				Minimum = result;
				Maximum = result2;
			}
			else
			{
				Minimum = ConvertValue(Minimum, formatProvider) ?? throw new InvalidOperationException("The minimum and maximum values must be set to valid values.");
				Maximum = ConvertValue(Maximum, formatProvider) ?? throw new InvalidOperationException("The minimum and maximum values must be set to valid values.");
			}
		}
		int num = ((IComparable)Minimum).CompareTo((IComparable)Maximum);
		if (num > 0)
		{
			throw new InvalidOperationException("The maximum value '{Maximum}' must be greater than or equal to the minimum value '{Minimum}'.");
		}
		if (num == 0 && (MinimumIsExclusive || MaximumIsExclusive))
		{
			throw new InvalidOperationException("Cannot use exclusive bounds when the maximum value is equal to the minimum value.");
		}
		Initialized = true;
	}
	if ((value == null || (value is string text && text.Length == 0)) ? true : false)
	{
		return true;
	}
	CultureInfo formatProvider2 = (ConvertValueInInvariantCulture ? CultureInfo.InvariantCulture : CultureInfo.CurrentCulture);
	object obj;
	if (OperandType == typeof(TimeSpan))
	{
		if (value is TimeSpan)
		{
			obj = value;
		}
		else
		{
			if (!(value is string))
			{
				DefaultInterpolatedStringHandler defaultInterpolatedStringHandler = new DefaultInterpolatedStringHandler(121, 1);
				defaultInterpolatedStringHandler.AppendLiteral("A value type ");
				defaultInterpolatedStringHandler.AppendFormatted(value!.GetType());
				defaultInterpolatedStringHandler.AppendLiteral(" that is not a TimeSpan or a string has been given. This might indicate a problem with the source generator.");
				throw new InvalidOperationException(defaultInterpolatedStringHandler.ToStringAndClear());
			}
			if (!TimeSpan.TryParse((string)value, formatProvider2, out var result3))
			{
				return false;
			}
			obj = result3;
		}
	}
	else
	{
		try
		{
			obj = ConvertValue(value, formatProvider2);
		}
		catch (Exception ex) when (((ex is FormatException || ex is InvalidCastException || ex is NotSupportedException) ? 1 : 0) != 0)
		{
			return false;
		}
	}
	IComparable comparable = (IComparable)Minimum;
	IComparable comparable2 = (IComparable)Maximum;
	if (MinimumIsExclusive ? (comparable.CompareTo(obj) < 0) : (comparable.CompareTo(obj) <= 0))
	{
		if (!MaximumIsExclusive)
		{
			return comparable2.CompareTo(obj) >= 0;
		}
		return comparable2.CompareTo(obj) > 0;
	}
	return false;
}

@martincostello
Copy link
Member

martincostello commented Jan 16, 2024

Looking at the code, there might be a race condition where the casts (string)Minimum or/and (string)Maximum fail if those values are already initialized with a TimeSpan value if Initialized hasn't been set to true yet if the validator runs concurrently. Source

@martincostello
Copy link
Member

One final data point, if I change my xunit configuration to have "parallelizeTestCollections": false then the issue goes away when I run all the tests in the same test run.

martincostello referenced this issue in martincostello/alexa-london-travel Jan 16, 2024
martincostello referenced this issue in martincostello/alexa-london-travel Jan 16, 2024
@jjanuszkiewicz
Copy link

parallelizeTestCollections helps here too.

@martincostello
Copy link
Member

martincostello commented Jan 16, 2024

This is where the exception comes from in my repro:

image

If I go up the stack enough, the value of value is a TimeSpan whose value is equal to 30 seconds:

image

Here's the state of the attribute, you can see that the Maximum and Minimum are assigned a TimeSpan value already and that Initialized is true.

image

@geeknoid
Copy link
Member

@tarekgh I think the InvalidCastException part of this problem has to do with the config validation code gen and how we're handling some racy initialization in the attribute. Can you take a look please?

@geeknoid
Copy link
Member

OK, so the problem is this:

  • Through the code generator, we're creating some static instances of the various validation attributes. In this case, we're creating a static instance of the RangeAttribute class.

  • We then call RangeAttribute.IsValid from multiple threads, since the expectation was that IsValid is a read-only operation and so can be safely called in parallel. Clearly, RangeAttribute.IsValid was not designed for parallel execution.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jan 16, 2024
@tarekgh tarekgh transferred this issue from dotnet/extensions Jan 16, 2024
@ghost
Copy link

ghost commented Jan 16, 2024

Tagging subscribers to this area: @dotnet/area-system-componentmodel-dataannotations
See info in area-owners.md if you want to be subscribed.

Issue Details

Problem with .AddStandardResilienceHandler(); when used in integration test with custom web factory and when there is more than one test.

I have a service that uses HttpClients to 3rd party endpoints. On this one, I use the.AddStandardResilienceHandler();
I set this up in my startup.cs then I have an integration test where I use the WebApplicationFactory

All works fine, but after some lib updates, I start getting random exceptions from my tests.
If I run one test at a time with no problem, it seems to be shared resource issues when they run in parallel.

The tests randomly give me:

  Message: 
System.ObjectDisposedException : Cannot access a disposed object.
Object name: 'IServiceProvider'.

And:

System.InvalidCastException: Unable to cast object of type 'System.TimeSpan' to type 'System.String'.
   at __OptionValidationGeneratedAttributes.<Validators_g>F8B1F3D42962A35D8FF1B1489612AEF6C36F3713335EFF79DB68A25973333C495____SourceGen__RangeAttribute.IsValid(Object value)
   at System.ComponentModel.DataAnnotations.ValidationAttribute.IsValid(Object value, ValidationContext validationContext)
   at System.ComponentModel.DataAnnotations.ValidationAttribute.GetValidationResult(Object value, ValidationContext validationContext)
   at System.ComponentModel.DataAnnotations.Validator.TryValidate(Object value, ValidationContext validationContext, ValidationAttribute attribute, ValidationError& validationError)
   at System.ComponentModel.DataAnnotations.Validator.GetValidationErrors(Object value, ValidationContext validationContext, IEnumerable`1 attributes, Boolean breakOnFirstError)
   at System.ComponentModel.DataAnnotations.Validator.TryValidateValue(Object value, ValidationContext validationContext, ICollection`1 validationResults, IEnumerable`1 validationAttributes)
   at Microsoft.Extensions.Http.Resilience.__HttpTimeoutStrategyOptionsValidator__.

It does not happen all the time but mostly every time.

I have tried fixture, using the same client that's created on all test methods but it seems like a disposal is happening.
But next time there is some expectation that the resilience services shall exist somehow. Not all services are not resettled.
Some singleton was added once I guess.

 public HttpClient CreateClient(ITestOutputHelper testOutputHelper = null)
 {
     _waf = new WebApplicationFactory<TProgram>()
        .WithWebHostBuilder(builder =>
        {

If I remove the .AddStandardResilienceHandler(); from my setup of the HTTP client all works fine.

I also tried to remove the added services and override them in

 builder.ConfigureTestServices(services =>
 {

But that will not work because it's still have added the resilience stuff before I reach this area.

So it seams like:
First it creates my client with resilience, then I ran the fist FACT test. Then a second one.
I have 2 tests one for success and one for failure. If I remove one of the test all works fine. But as soon I share the client in many test this happens.

Not sure if there is som clean-up, or dispose issues with the resilience setup? I have followed the basic guidelines to setup integration tests in .net

Have I missed something? Or is this a known bug?
I can see there is a bug reported where it seams to be a shared setting for all services atm so we can't override settings on different once. Maybe that's related here?

Author: jnormen
Assignees: -
Labels:

bug, area-System.ComponentModel.DataAnnotations, untriaged

Milestone: -

@tarekgh
Copy link
Member

tarekgh commented Jan 16, 2024

I moved the issue to the runtime repo. I'll take a look.

@tarekgh
Copy link
Member

tarekgh commented Jan 16, 2024

@martincostello is it possible you can share the source generated file from your repro?

@martincostello
Copy link
Member

I'm not at a computer until tomorrow now, but I had to use ILSpy to get the code because it was compiled into the Resilience library, rather than being generated into my own project.

@tarekgh tarekgh added area-Extensions-Options source-generator Indicates an issue with a source generator feature and removed area-System.ComponentModel.DataAnnotations labels Jan 16, 2024
@ghost
Copy link

ghost commented Jan 16, 2024

Tagging subscribers to this area: @dotnet/area-extensions-options
See info in area-owners.md if you want to be subscribed.

Issue Details

Problem with .AddStandardResilienceHandler(); when used in integration test with custom web factory and when there is more than one test.

I have a service that uses HttpClients to 3rd party endpoints. On this one, I use the.AddStandardResilienceHandler();
I set this up in my startup.cs then I have an integration test where I use the WebApplicationFactory

All works fine, but after some lib updates, I start getting random exceptions from my tests.
If I run one test at a time with no problem, it seems to be shared resource issues when they run in parallel.

The tests randomly give me:

  Message: 
System.ObjectDisposedException : Cannot access a disposed object.
Object name: 'IServiceProvider'.

And:

System.InvalidCastException: Unable to cast object of type 'System.TimeSpan' to type 'System.String'.
   at __OptionValidationGeneratedAttributes.<Validators_g>F8B1F3D42962A35D8FF1B1489612AEF6C36F3713335EFF79DB68A25973333C495____SourceGen__RangeAttribute.IsValid(Object value)
   at System.ComponentModel.DataAnnotations.ValidationAttribute.IsValid(Object value, ValidationContext validationContext)
   at System.ComponentModel.DataAnnotations.ValidationAttribute.GetValidationResult(Object value, ValidationContext validationContext)
   at System.ComponentModel.DataAnnotations.Validator.TryValidate(Object value, ValidationContext validationContext, ValidationAttribute attribute, ValidationError& validationError)
   at System.ComponentModel.DataAnnotations.Validator.GetValidationErrors(Object value, ValidationContext validationContext, IEnumerable`1 attributes, Boolean breakOnFirstError)
   at System.ComponentModel.DataAnnotations.Validator.TryValidateValue(Object value, ValidationContext validationContext, ICollection`1 validationResults, IEnumerable`1 validationAttributes)
   at Microsoft.Extensions.Http.Resilience.__HttpTimeoutStrategyOptionsValidator__.

It does not happen all the time but mostly every time.

I have tried fixture, using the same client that's created on all test methods but it seems like a disposal is happening.
But next time there is some expectation that the resilience services shall exist somehow. Not all services are not resettled.
Some singleton was added once I guess.

 public HttpClient CreateClient(ITestOutputHelper testOutputHelper = null)
 {
     _waf = new WebApplicationFactory<TProgram>()
        .WithWebHostBuilder(builder =>
        {

If I remove the .AddStandardResilienceHandler(); from my setup of the HTTP client all works fine.

I also tried to remove the added services and override them in

 builder.ConfigureTestServices(services =>
 {

But that will not work because it's still have added the resilience stuff before I reach this area.

So it seams like:
First it creates my client with resilience, then I ran the fist FACT test. Then a second one.
I have 2 tests one for success and one for failure. If I remove one of the test all works fine. But as soon I share the client in many test this happens.

Not sure if there is som clean-up, or dispose issues with the resilience setup? I have followed the basic guidelines to setup integration tests in .net

Have I missed something? Or is this a known bug?
I can see there is a bug reported where it seams to be a shared setting for all services atm so we can't override settings on different once. Maybe that's related here?

Author: jnormen
Assignees: -
Labels:

bug, untriaged, area-Extensions-Options, source-generator

Milestone: -

@tarekgh tarekgh removed the untriaged New issue has not been triaged by the area owner label Jan 16, 2024
@tarekgh tarekgh self-assigned this Jan 16, 2024
@tarekgh tarekgh added this to the 8.0.x milestone Jan 16, 2024
@tarekgh
Copy link
Member

tarekgh commented Jan 16, 2024

I'm not at a computer until tomorrow now, but I had to use ILSpy to get the code because it was compiled into the Resilience library, rather than being generated into my own project.

never mind. I have created an isolated simple repro and I can see the issue now.

@dominikjeske
Copy link

@tarekgh PR is in mailstone 9 - can we hope to get in in 8.0.2?

@tarekgh
Copy link
Member

tarekgh commented Jan 17, 2024

PR is in mailstone 9 - can we hope to get in in 8.0.2?

I intend to port the fix to version 8.0 after merging it into the main branch. This will likely be included in 8.0.3, as the cutoff for 8.0.2 has already passed.

@ghost ghost added in-pr There is an active PR which will close this issue when it is merged and removed in-pr There is an active PR which will close this issue when it is merged labels Jan 17, 2024
@tarekgh
Copy link
Member

tarekgh commented Jan 18, 2024

I have submitted the fix to main and 8.0 servicing branches. Hopefully, this will be included in the February 8.0 servicing release.

@tarekgh tarekgh closed this as completed Jan 18, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Feb 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Extensions-Options bug source-generator Indicates an issue with a source generator feature
Projects
None yet
Development

No branches or pull requests

7 participants