Skip to content
This repository has been archived by the owner on Dec 19, 2018. It is now read-only.

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

Closed
Rinsen opened this issue Jul 22, 2018 · 11 comments

Comments

@Rinsen
Copy link

Rinsen commented Jul 22, 2018

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
aspnet/Logging#701
That was similar/same as this issue
#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
Copy link
Member

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

@Rinsen
Copy link
Author

Rinsen commented Jul 22, 2018

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
Copy link
Author

Rinsen commented Jul 24, 2018

@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
Copy link
Member

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
Copy link

bruno-garcia commented Jul 29, 2018

@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
Copy link
Author

Rinsen commented Jul 29, 2018

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

@bruno-garcia
Copy link

bruno-garcia commented Jul 29, 2018

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
Copy link

schuettecarsten commented Aug 8, 2018

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 dotnet/aspnetcore#3395

@muratg muratg added this to the Discussions milestone Aug 28, 2018
@bruno-garcia
Copy link

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
Copy link
Member

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
Copy link

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.

@aspnet-hello aspnet-hello removed this from the Discussions milestone Dec 18, 2018
@aspnet aspnet locked and limited conversation to collaborators Dec 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants