aspnet / Hosting

[Archived] Code for hosting and starting up an ASP.NET Core application. Project moved to https://github.com/aspnet/Extensions and https://github.com/aspnet/AspNetCore
Apache License 2.0
552 stars 312 forks source link

New issue with two custom ILoggerProvider singletons created on application startup introduced in 2.1 #1499

Closed Rinsen closed 5 years ago

Rinsen commented 6 years ago

I have had an similar issue earlier but this is not exactly the same and I can't figure out how to do some workaround this time.

My first issue in 2.0 https://github.com/aspnet/Logging/issues/701 That was similar/same as this issue https://github.com/aspnet/Hosting/issues/1150

Here I have a repo of the issue with the latest commit showing the change between 2.1 and 2.0 removing the part that introduced the issue in 2.0. https://github.com/Rinsen/MultipleSingletonsCreated

The instance is created for this logger type "Microsoft.AspNetCore.Hosting.Internal.WebHost"

The easiest way to understand the repo is to put a breakpoint in the ctor in DoNothingLogProvider and end up there two times

davidfowl commented 6 years ago

Yea this is by design. Are you running into something specific?

Rinsen commented 6 years ago

I Always get two singletons when I expect to only get one. It was possible previously to only get one by not injecting it to the Startup class.

But maybe I just have to live with two instances?

The reason why It's a bit confusing is that I have end up with two background workers pushing logs to a log service with one queue for each and when debugging things I may end up with looking in the wrong queue. So it's no major issue, just a bit confusing.

Rinsen commented 6 years ago

@davidfowl

Should I maybe just close this issue? No reason to have an open issue hanging around for a thing that is by design and will not change. I don't need more answers as the cost for having a worker doing more or less nothing is so small that it doesen't matter at all.

davidfowl commented 6 years ago

Right so I believe what you are seeing is related to the fact that in 2.1, we allow 3rd party containers to customer Startup activation and that basically requires us to resolve things earlier in another bootstrapping container.

bruno-garcia commented 6 years ago

@davidfowl even when no 3rd party container is used this behavior is valid?

Like @Rinsen, this affected me where I expected a singleton to initialize some behavior and ended up having two.

I wrote a repro, before I saw this issue was already open. Really felt like a bug/No idea it was by design. https://github.com/bruno-garcia/repro-logger-provider-double-instantiation

Rinsen commented 6 years ago

@bruno-garcia I use only the built in container so it's not only with 3rd party containers so probably.

bruno-garcia commented 6 years ago

I'm reproducing a problem where two instances are created but only the second one is disposed. This really affects me since the first instance ends up doing the initialization of some state which needs to be disposed and that never gets disposed.

Is the reason of the two instance some intermediate CreateServiceProvider called by the GenericHostBuilder? If so, shouldn't that call dispose on that intermediate container?

This only happens under TestServer though, it works fine on the real app.

schuettecarsten commented 6 years ago

Another problem is that - when two LoggingProviders are instantiated, the second one has a different external scope provider. The application uses the second instance for logging, but the WebHost seems to use the external scope provider of the first, orphaned logging provider. In this case, add logging scope details from WebHost are lost (ConnectionId, RequestId).

I just reported this as https://github.com/aspnet/Home/issues/3395

bruno-garcia commented 6 years ago

It seems that both containers are disposed on app shutdown. Is that the case? Does the first container just stays in memory and is not used?

Is this behavior documented? It would be great to understand it a bit better as it's side effect really affects me.

davidfowl commented 5 years ago

It seems that both containers are disposed on app shutdown. Is that the case?

Yes, they are disposed when the host is disposed.

Does the first container just stays in memory and is not used?

Correct, it's used to bootstrap the Startup class and nothing more.

Is this behavior documented? It would be great to understand it a bit better as it's side effect really affects me.

Probably, not, it's pretty hairy and we plan to revamp this behavior in 3.0.

aspnet-hello commented 5 years ago

We periodically close 'discussion' issues that have not been updated in a long period of time.

We apologize if this causes any inconvenience. We ask that if you are still encountering an issue, please log a new issue with updated information and we will investigate.