JasperFx / lamar

Fast Inversion of Control Tool and Successor to StructureMap
https://jasperfx.github.io/lamar
MIT License
563 stars 118 forks source link

Config change led to "Message=Cannot access a disposed object." #319

Closed JohnBall007 closed 2 years ago

JohnBall007 commented 2 years ago

We migrated to Lamar from StructureMap a few months ago and were delighted by the time reduction in our regular integration tests from an hour to 6 minutes. We are now running Lamar 7.1.1 on .NET 6.0. There are two questions I have which I can't see in the sample documentation stemming from our need to load a DB first.

(1) To include Lamar, we added the extension method into program.cs .UseLamar(new LamarProdRegistry()) to incorporate our existing registry file. It has been working fine and still drives tests without error, but it is unclear what the best way is to create an additional object for prod. We have a few singletons in the registry to configure and not respecting that seemed to violate the lifecycle (validly). Is there a way to run the validation for Lamar after the initiation? .NET 6.0 doesn't make the approach obvious.

(2) Before our webapi starts, we want to load a custom DB into memory, and have added an async task. This seems a bad idea, as now we get constant errors about accessing a disposed object, especially with Func creation. Is there an approach recommended for Lamar users? A sample message is: "System.ObjectDisposedException HResult=0x80131622 Message=Cannot access a disposed object. Object name: 'This Container has been disposed'. Source=Lamar StackTrace: at Lamar.IoC.Scope.assertNotDisposed() at Lamar.IoC.Scope.GetInstance(Type serviceType) at Lamar.IoC.Scope.GetInstance[T]() "

We are back on this tomorrow. I hope this is a valid way to raise a question!

JohnBall007 commented 2 years ago

I'm working through https://stackoverflow.com/questions/32459670/resolving-instances-with-asp-net-core-di-from-within-configureservices which seems comprehensive. The comment: "warning if you need to resolve services in ConfigureServices and that service is a singleton it will be a different singleton to the one your Controllers use!" was one of my observations. Fixing that by creating static access just moved the problem to this one above. I think I just need to define a common container used by both the controllers and my infrastructure/memory-loaded database.

JohnBall007 commented 2 years ago

Last attempt to diagnose the problem today. All our tests run using the current code base and Lamar. Only use via the controllers and the system startup is under investigation. When we load the system via the first controller call (injecting IServiceProvider and using it to create our loading object or just injecting the loading provider directly), the system loads as usual, but we then get the runtime errors as above. The system used to work (before and after .net 6.0 migration) by doing a "manual load" including creating and loading a container as we do for testing.

jeremydmiller commented 2 years ago

@JohnBall007

1.) I might need a little more context here. Are you wanting to run the Lamar container diagnostics after the app is started? That's easy enough w/ Lamar.Diagnostics + Oakton as explained in the Lamar docs. But I'm not sure that's what you're asking here.

2.) Your code is erroneously disposing the main container somewhere in the async code most likely. A workaround is to set the Container.DisposalLock property to latch that. I can't think of a super clean way to do that w/ the HostBuilder mechanics though. You could always do that from an IHostedService that takes in IContainer. That might be worth a PR to add that into Lamar. Really just a temporary hack though to help you find the real culprit.

JohnBall007 commented 2 years ago

@jeremydmiller Thanks for your update.

1.) Our testing works perfectly, which looks like a normal structuremap/lamar bootstrap. Here we create a container, and then create a single object that bootstraps our system - basically a memory-based repository with access via singletons. image I was looking for a seam with the .NET 6.0 startup process where I can access the container that is created as a final step.

I just want to replicate the test implementation. I don't think that was explained well, but does this help?

I've tried a few things to debug. Converting the singletons to static moved the problem to the creation with Func. I think we hit compatability issues with the prod startup due to the old code. There are concrete classes that have no interfaces that happily resolve today, but there seemed a change by adding an interface and adding to the registry. There is also some horrible recursive code that creates new objects on the fly. I can look to rework that to modern standards, anyway, to see if it changes the symptoms.

To find a seam to bootstrap our system, I grabbed an IServiceProvider from a lazy controller call. Now presumably the Lamar.Container found there can't be disposed but using that I create the system, but the rest of the code will be disposed. Controller: image System generator: image

2.) I've eliminated the async calls for the moment to isolate the problem. We still have the failure and in reality if we find a seam or way to bootstrap within the .NET 6.0 startup, perhaps that can be removed from the design. I think we can do more to isolate the problem at this end (and that will give me time to read up on the PR process!)

I hope this is enough context to explain what we are doing, and perhaps you can see the obvious mistake we are making! Again, many thanks for your assistance!

JohnBall007 commented 2 years ago

End-of-day status: I have refactored some of the old code that I was suspicious of, but basically removing any of the failing Func elements simply moves the problem to the next Func. Outside of the registry definition and the startup/program files, there are no references to Lamar in the code base. It's unclear why we have no other DI problems that complain about the lost container other than the Func. I'm checking with my team how to raise the PR correctly so we can try to workaround to isolate the problem.

jeremydmiller commented 2 years ago

Ah, because the Func would be wrapping the container that created it, so if you pulled that out of a nested container and kept it around longer than the nested container itself, I'd expect it to blow up exactly like that. Jeremy D. Miller The Shade Tree Developer @.***

On Tuesday, January 25, 2022, 05:08:22 AM CST, John Ball ***@***.***> wrote:  

End-of-day status: I have refactored some of the old code that I was suspicious of, but basically removing any of the failing Func elements simply moves the problem to the next Func. Outside of the registry definition and the startup/program files, there are no references to Lamar in the code base. It's unclear why we have no other DI problems that complain about the lost container other than the Func. I'm checking with my team how to raise the PR correctly so we can try to workaround to isolate the problem.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

JohnBall007 commented 2 years ago

We've had some change with the startup process by putting it in a scoped wrapper, but the Func problem continues. We refactored to remove that call from some code as an immediate solution and for further diagnosis, but the next step is to get an answer from the community on how get something equivalent to our testing startup that works through the minimal hosting setup the same way. Re: the PR to diagnose with Container.DisposalLock , I have done a fork on the Lamar branch and will create the pull request after adding a branch. Is that the right approach (I've not done that before and presume there is no actual code I need to touch!)

JohnBall007 commented 2 years ago

Well, we now have a workaround that is using a second container. That suggests that the problem is with the disposal of the Func presumably for lifecycle. But we haven't isolated properly yet. It seems that only the Func fails where the classes are storing them as fields. Maybe that isn't valid? Or maybe the representation of the field is disposed by .NET's DI, but we retain the same transient class. I don't know yet, but we will test the theories.

JohnBall007 commented 2 years ago

Nearly there @jeremydmiller . We now have a fix that is just an addition to the registry. Whoo hoo!

Problem summary: We track state using a singleton in our code-base. User-state can last a long time. But we access that state with an object graph from the controller that includes an injected Func as a factory. I took Func as a primitive, but if we make it a singleton, we don't lose it. I presume we lose it at present because it gets disposed as a transient injection during the controller transient lifecycle, even though the object that holds it remains connected to our state. Maybe we violate the lifecycle rules...

Is this expected behaviour (assuming my analysis is confirmed?) that the DI treats the Func<> as a transient injected object, so when the single use of the class completes, it is disposed, even though a class actively holds its object handle. Reason: Lamar caters to Func<>, but .NET CORE does not?

Here's our definition with one of the Func being made into a singleton (per https://stackoverflow.com/questions/35736070/how-to-use-funct-in-built-in-dependency-injection).

image

jeremydmiller commented 2 years ago

@JohnBall007 I understand what's happening with the Func, and I did try to explain that above. The simple workaround is to not use Func from Lamar as a long lived factory. Honestly, just inject IContainer or IServiceProvider into your singleton and use that so you have more control. Or if you care about hiding that coupling, roll a factory service that wraps IContainer/IServiceProvider.

And if you're in business, I'm closing this.

JohnBall007 commented 2 years ago

Thanks @jeremydmiller I appreciate your guidance on this. It must be challenging dealing with inexperienced people like me, but this final response has directed me to a solution. I was fixated on getting the Func to work rather than just solving the problem. We have a codebase with a few factories of this nature and finding a silver bullet (like this) was preferred to migration.