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

Failure to capture event: System.ObjectDisposedException: Cannot access a disposed object. #103

Closed
alesdvorakcz opened this issue Sep 22, 2018 · 37 comments
Labels
Bug Something isn't working

Comments

@alesdvorakcz
Copy link

alesdvorakcz commented Sep 22, 2018

Hi I tried sentry in my asp.net core app. When I turn on Debug mode I can see this exception:

Sentry.ISentryClient[0]
      Failure to capture event: System.ObjectDisposedException: Cannot access a disposed object.
      Object name: 'SentryClient'.
         at Sentry.SentryClient.CaptureEvent(SentryEvent event, Scope scope) in C:\projects\sentry-dotnet\src\Sentry\SentryClient.cs:line 77
         at Sentry.Internal.Hub.CaptureEvent(SentryEvent evt, Scope scope) in C:\projects\sentry-dotnet\src\Sentry\Internal\Hub.cs:line 95
dbugdbug: Sentry.ISentryClient[0]
      Configuring the scope.
: Sentry.ISentryClient[0]
      Configuring the scope.
dbugfail: Sentry.ISentryClient[0]
      Configuring the scope.
: Sentry.ISentryClient[0]
      Failure to capture event: System.ObjectDisposedException: Cannot access a disposed object.
      Object name: 'SentryClient'.
         at Sentry.SentryClient.CaptureEvent(SentryEvent event, Scope scope) in C:\projects\sentry-dotnet\src\Sentry\SentryClient.cs:line 77
         at Sentry.Internal.Hub.CaptureEvent(SentryEvent evt, Scope scope) in C:\projects\sentry-dotnet\src\Sentry\Internal\Hub.cs:line 95

I dont think I have some special setup. Its dotnet core 2.1 web api with EF core and Autofac DI container.
Unfortunately I can post here whole project but if you need more info don't hesitate to ask.

@bruno-garcia bruno-garcia added the Bug Something isn't working label Sep 26, 2018
@bruno-garcia
Copy link
Member

@alesdvorakcz
This would happen in case the SDK got disposed.

Could you please clarify how do you initialize the SDK? Is it via SentrySdk.Init? If so, how do you handle the IDisposable returned?

Or do you use UseSentry on the WebHostBuilder like the samples?

Did you register ISentryClient or IHub with an Autofac module or something? It could be an issue with the lifetime used in the registration.

It would be great if you could have a small reproducible code to make it easier to troubleshoot.

@bruno-garcia bruno-garcia added the Question Further information is requested label Sep 26, 2018
@alesdvorakcz
Copy link
Author

I dont use Static API.

This is my WebHostBuilder:

WebHost.CreateDefaultBuilder(args)
                .ConfigureAppConfiguration((context, builder) =>
                {
                    builder
                        .SetBasePath(context.HostingEnvironment.ContentRootPath)
                        .AddJsonFile("appsettings.json", optional: false, reloadOnChange: true)
                        .AddJsonFile($"appsettings.{context.HostingEnvironment.EnvironmentName}.json", optional: true)
                        .AddEnvironmentVariables();
                })
                .ConfigureServices(services =>
                {
                    services.AddSingleton<IStartup, Startup>();
                })
                .UseSentry()
                .Build()
                .Run();

In config I have only

{
...
  "Sentry": {
    "Dns": "..."
  }
}

Here is my Startup.ConfigureServices where I integrate Autofac:

public IServiceProvider ConfigureServices(IServiceCollection services)
{
  var containerBuilder = new ContainerBuilder(); //AutoFac ContainerBuilder

  services.Configure<AzureBlobConfig>(Configuration.GetSection("AzureBlob"));
  ConfigureIdentityServer(services);
  RegisterDbContext(services);
  RegisterMvc(services);
  RegisterSwagger(services);
  IoCHelper.ConfigureContainer(containerBuilder);

  containerBuilder.Populate(services);

  Container = containerBuilder.Build();
  return Container.Resolve<IServiceProvider>();
}

I dont register a client, hub or anything by my own so I dont set any lifetimes to Sentry.

@bruno-garcia
Copy link
Member

@alesdvorakcz I'm not able to reproduce this issue.
I added Autofac to one of the samples:

590cf89

Running it I see:

trce: Sentry.AspNetCore.SentryMiddleware[0]
      Sending event 'Sentry.SentryEvent' to Sentry.
info: Sentry.AspNetCore.SentryMiddleware[0]
      Event '5259a129-ff0e-4b44-b55a-5dc71e3055e9' queued.

Could you please create a small reproducible repo? I could check that out and debug through.

@alesdvorakcz
Copy link
Author

I will try to locate problem more specifically and send you small repo. Thanks

@klemmchr
Copy link

Can confirm this bug. We are using AspNetCore 2.1 with Ninject. We integrated Ninject following this tutorial. I'm quite sure there is a correlation between the custom DI solution and this error. We also just used the extension method for the WebHostBuilder.

fail: Sentry.ISentryClient[0]
      Failure to capture event: System.ObjectDisposedException: Cannot access a disposed object.
      Object name: 'SentryClient'.
         at Sentry.SentryClient.CaptureEvent(SentryEvent event, Scope scope) in C:\projects\sentry-dotnet\src\Sentry\SentryClient.cs:line 77
         at Sentry.Internal.Hub.CaptureEvent(SentryEvent evt, Scope scope) in C:\projects\sentry-dotnet\src\Sentry\Internal\Hub.cs:line 107
14:21:41.425 [1] ERROR Sentry.ISentryClient - Failure to capture event: System.ObjectDisposedException: Cannot access a disposed object.
Object name: 'SentryClient'.
   at Sentry.SentryClient.CaptureEvent(SentryEvent event, Scope scope) in C:\projects\sentry-dotnet\src\Sentry\SentryClient.cs:line 77
   at Sentry.Internal.Hub.CaptureEvent(SentryEvent evt, Scope scope) in C:\projects\sentry-dotnet\src\Sentry\Internal\Hub.cs:line 107
14:21:41.431 [1] ERROR Sentry.ISentryClient - Failure to capture event: System.ObjectDisposedException: Cannot access a disposed object.
Object name: 'SentryClient'.
   at Sentry.SentryClient.CaptureEvent(SentryEvent event, Scope scope) in C:\projects\sentry-dotnet\src\Sentry\SentryClient.cs:line 77
   at Sentry.Internal.Hub.CaptureEvent(SentryEvent evt, Scope scope) in C:\projects\sentry-dotnet\src\Sentry\Internal\Hub.cs:line 107

@bruno-garcia
Copy link
Member

bruno-garcia commented Oct 19, 2018

@chris579 it seems to be related then to using a third party DI container which wasn't tested.
I've made a branch testing Autofac and could not reproduce the issue.

Could you please create a small reproducible repo? It would really help pinpoint the issue and fix it.

It seems IHub or ISentryClient gets resolved from a scope which gets disposed.
This shouldn't be the case since it's registered in the root container as Singleton. It should only be disposed when shutting down the app.

@klemmchr
Copy link

I'll try to create a reproduction sample but the repro I created is working fine. I'll further investigate this issue by taking the Sentry source into my project and debug the client the next days.

@alesdvorakcz
Copy link
Author

I tried to made small repo to isolate problem but it works fine there... maybe it has some connection with other asp services I use (EF Core, ...). I will try to find problem and then I will post it.

@prencher
Copy link

prencher commented Oct 25, 2018

Just chiming in to say we also see it with .NET Core 2.0 and Sentry 1.0.0, with exactly the same kind of traces as the initial post, and also simply using UseSentry with a simple Dsn definition in appsettings.json.

A wrinkle is that it only happens when using a dependency injected logger, e.g. passed to a controller using FooController(ILogger<FooController> logger).

When doing Services.GetRequiredService<ILogger<FooController>>().LogError(ex, "test"), everything works as expected, but you then lose all request context.

bruno-garcia added a commit that referenced this issue Oct 25, 2018
* match original issue description #103
@bruno-garcia
Copy link
Member

@prencher I've pushed this branch: https://github.com/getsentry/sentry-dotnet/tree/repro/autofac
Where I plugged in Autofac.
In HomeController.cs a ILogger<> is also injected and works as expected.

I can't reproduce this error. It's affecting different setups as we can see in this thread (Ninject, Autofac) but somehow I still can't get to repro the problem in order to fix it.

Would it be possible for someone who can repro this issue to strip off from the solution whatever can't be made public and share the code so we can debug through it?

@bruno-garcia bruno-garcia removed the Question Further information is requested label Oct 25, 2018
@klemmchr
Copy link

Just to note that here: we are using the Log4net AspNetCore Integration. I'll try to add logging to my repro attempt, maybe this is causing the bug.

@prencher
Copy link

@bruno-garcia For us, we don't use autofac, just standard .NET Core 2.0 with ASP.NET Core 2.0.9, and no other logging providers except for the standard console & debug ones.

When enabling Sentry debug, I can see two separate cases of it starting up, and then one shutting itself down and disposing, which seems to be the one used in the controller DI loggers.

I'm actually traveling until mid next week, but I'll try to create a minimal reproduction when back if necessary.

@klemmchr
Copy link

klemmchr commented Oct 29, 2018

When enabling Sentry debug, I can see two separate cases of it starting up, and then one shutting itself down and disposing, which seems to be the one used in the controller DI loggers.

Can confirm that, did the same observations when stepping through the code.

I managed to tackle the issue down to Swagger (which is absurdly strange) but I'm still unable to reproduce it in a separate project.

Would it be possible for someone who can repro this issue to strip off from the solution whatever can't be made public and share the code so we can debug through it?

For me this is not possible. Our project is way too big to be able to strip out business relevant parts.

@bruno-garcia
Copy link
Member

bruno-garcia commented Oct 29, 2018

When enabling Sentry debug, I can see two separate cases of it starting up, and then one shutting itself down and disposing, which seems to be the one used in the controller DI loggers.

@chris579 @prencher the two initializations are due to ASP.NET Core creating two containers.
It creates an intermediary which gets disposed. There's an issue raised but it's by design: aspnet/Hosting#1499

Also, Damian Edwards mentions it yesterday in the [community standup].(https://www.youtube.com/watch?v=-b59KvkWBUo&feature=youtu.be&t=1798)

For us what this means is that: If an exception happens when the app is starting, depending where it happens the SDK will capture that error and report to Sentry. If no error happens during that first container's lifetime, it gets disposed while disposing the SDK and a new initialization happens after with the final container which exists for the lifetime of the app.

This is the behaviour also in the samples in this repo which don't reproduce the error reported in this issue.

That said I still haven't been able to reproduce the issue. Still don't understand how it happens either.

@prencher
Copy link

We also use Swagger, but disabling it does not fix the issue for us, so I think that's a red herring.

@ryno1234
Copy link

ryno1234 commented Nov 1, 2018

I'm having this same issue. For me, it is a difference between running on Linux vs Windows. I develop on Windows but pushed to Linux. As far as I can tell, everything else is the same between the two deployments but the Linux deployment is throwing this same Object Disposed error.


UPDATE: Welp, scratch that - It's no longer giving me issues for some reason on my Linux system after restarting the process.

@alesdvorakcz
Copy link
Author

Can confirm problem is related to Swagger (I am not sure if this is only problem but was the first one I was able to reproduce in small repo)
https://github.com/alesdvorakcz/sentry-test

Last commit (added swagger) started to make issues. Before it was working fine.

Hope it helps

bruno-garcia added a commit that referenced this issue Nov 5, 2018
@bruno-garcia
Copy link
Member

bruno-garcia commented Nov 5, 2018

@alesdvorakcz I added Swagger in the commit linked above and still couldn't repro.

I've tried your repro (thank you very much for it) and I could find the culprit:

var provider = services.BuildServiceProvider().GetRequiredService<IApiVersionDescriptionProvider>();

This builds a container which re-initializes the SDK which disposes the previously initialized instance.
The disposed instance is still around in the real container, used by the application where the client created by this intermediary container can only be accessed through: SentrySdk

The reason my attempt to reproduce the issue with swagger didn't work is that I don't build a container like in your sample.

Is this the documented way to add things to SwaggerDoc? Creating a container like could have side effects (like in this case). Also, note that the container should be disposed (unrelated to this issue though).

Regardless if this is the documented way (IMHO Swashbuckle should expose an overload that passes the ServiceProvider as an argument to the callback instead) we need to find a work around to this.

@klemmchr
Copy link

klemmchr commented Nov 5, 2018

Is this the documented way to add things to SwaggerDoc? Creating a container like could have side effects (like in this case). Also, note that the container should be disposed (unrelated to this issue though).

Yes, it is. It even is provided as an example from Microsoft for API Versioning with Swashbuckle.

Regardless if this is the documented way (IMHO Swashbuckle should expose an overload that passes the ServiceProvider as an argument to the callback instead) we need to find a work around to this.

Yep. Swashbuckle really needs a solution that doesn't require to build a ServiceProvider during initialization.

My temporary workaround is to create a property ServiceProvider in Startup and assign it at the top of Configure().

ServiceProvider = app.ApplicationServices;

The initialization of swagger is happening afterwards then and no additional container is build.

@bruno-garcia
Copy link
Member

@chris579 glad to hear there's a work around!

@ryno1234
Copy link

ryno1234 commented Nov 5, 2018

This problem is back for me. I do not have Swagger installed, but I am deploying to a Linux platform. For me, that is the difference between Sentry functioning or not functioning properly (works fine on Windows).

I was thinking I might be able to debug this by setting a breakpoint at the Dispose method of the Sentry client and tracing back the reason for the call.

Any insight?

@bruno-garcia
Copy link
Member

@ryno1234 either you or one of your dependencies is likely building an intermediary container like:

services.BuildServiceProvider()

Details are described here: #103 (comment)

@ryno1234
Copy link

ryno1234 commented Nov 5, 2018

@bruno-garcia , you're absolutely correct. I myself am doing that in my startup code. Odd that sometimes Sentry works fine and other times it does not. Perhaps due to some race condition....

At any rate, do you have a suggested work-around? I have services being registered during startup that I need access to within other startup Func<> / Action<> bodies i.e.

string connectionString = this.Configuration.GetValue<string>("ConnectionString");

// other service configurations go here
services.AddDbContextPool<MyDbContext>(
	options => options.UseMySql(connectionString)
);

// We need to use the service provider to access the DB context a little further down in the startup code, so build a service provider now.
var intermediateServiceProvider = services.BuildServiceProvider();

// Add authentication services
services.AddOpenIdConnect("AuthProvider", options =>
{
	// Abbreviated....
	
	options.Events = new OpenIdConnectEvents
	{
		OnTicketReceived = (context) => {
			var dbContext = intermediateServiceProvider.GetRequiredService<MyDbContext>();
			
			// Do stuff with DB context.
		}
	};
}

@klemmchr
Copy link

klemmchr commented Nov 5, 2018

@ryno1234 my workaround should apply to your case too. Your access on the DbContext should happen after init.
There is a race condition occurring that results in the manner that it’s working sometimes.

I‘m quite unsure why Sentry is being disposed when a new ServiceProvider is built. It’s not uncommon (especially for EntityFrameworkCore scenarios) that it is being built additionally to access the DI. IMO there is no reason why the Sentry Client has to behave like a singleton across different ServiceProviders.

@bruno-garcia
Copy link
Member

bruno-garcia commented Nov 5, 2018

@chris579 some integrations are not built with DI in mind and need access to the SentrySdk static class. When a Hub instance is first resolved, it creates the instance (initializes the SDK) and binds it to the SentrySdk static class so that the IHub or ISentryClient you get injected somewhere is the same being accessed through SentrySdk.

This makes sure that breadcrumbs and other things added to the scope are shared by the static context (SentrySdk.AddBreadcrumb) and also via DI like the SentryMiddleware's instance of Hub.

One example is Hangfire which I haven't had time to build an integration yet but would need to rely on SentrySdk.

@klemmchr
Copy link

klemmchr commented Nov 5, 2018

This does not explain why the old sentry client is disposed then. Whatever, this issue has to be resolved by Sentry itself as a lot of common cases are building the ServiceProvider manually to acces the DI.

@prencher
Copy link

prencher commented Nov 5, 2018

That is indeed the issue, we do this at the end of our ConfigureServices call: Services = services.BuildServiceProvider();, which we have to use a in a few places where we can't currently do DI. This seems like a shortcoming of Sentry, not something that should require a workaround.

@bruno-garcia
Copy link
Member

bruno-garcia commented Nov 6, 2018

@prencher as I mentioned above, we'll need to work around this issue in the SDK itself.

The proposed work around in this thread for you is only until we ship a new version which no longer requires you to work around this issue.

One way I'm thinking we can work around it is simply bind Hub to SentrySdk in the SentryMiddleware. In that case even if multiple containers are built, the last one to resolve the application pipeline is made sure to be the one bound to SentrySdk.

@alesdvorakcz
Copy link
Author

Ok thanks to you I am using workaround now and it works. My second question related to my sample repo is why exception sended by Microsoft.Extensions.Logging.ILogger.LogError() is not sent to sentry? I have debug mode turned on and I can see only unhandled exception is sent to Sentry but exception catched in BaseController and logged by ILooger is not. Is that correct behavior or not?

@bruno-garcia
Copy link
Member

By default, if you are not using a logging library that removes our provider, such as Serilog, LogError sends event unless you configure MinimumEventLevel to something else.

There's a PR open for Serilog support already. It will be merged soon.

@alesdvorakcz
Copy link
Author

Can you please check that repo? https://github.com/alesdvorakcz/sentry-test
I am not using anything more than .UseSentry() extension method and buildin logger. No extra configuration is provided

@prencher
Copy link

prencher commented Dec 4, 2018

Any traction for this issue being fixed in 1.0.1? This is unfortunately severely limiting the context we get from Sentry, because only the logging integration works, not the ASP.NET part.

@bruno-garcia
Copy link
Member

Release 1.1 just went out and this bug was not fixed yet. Apologies!

I hope to take care of this sooner rather than later but besides the smaller fixes and features from 1.1 I've been busy with other things outside the .NET SDK.

@bruno-garcia
Copy link
Member

A late Christmas miracle! Took a while but finally released a package with a fix for this issue.

The SDK will no longer hold on to the last Hub it creates via the DI container.
Instead, it will use an accessor (Func<IHub>) so if your app or some framework requires building containers after the SDK is initialized, the new Hub will just take over. That is, so any integration you have enabled (say ASP.NET Core and Serilog, for example) will share state.

One example: crumbs added via the Serilog integration are sent out if the ASP.NET Core integration captures an exception, or vice-versa.

The release is version 1.1.2-beta. Please give it a try and let give some feedback if there's anything wrong.

https://www.nuget.org/packages/Sentry.AspNetCore/1.1.2-beta

Thanks everyone who helped tracking the error and @alesdvorakcz for giving the repro!

@prencher
Copy link

Just wanted to leave a quick followup now that I had some cycles to go back and upgrade our main service using the new SDK: Works great! Thanks so much.

@bruno-garcia
Copy link
Member

@Precher thanks for being so patient for this fix. This one took a while, unfortunately.

@shivasuri
Copy link

Hi, I'm running into this same issue for a WinForms app. Is there a solution for WinForms?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants