simpleinjector / SimpleInjector

An easy, flexible, and fast Dependency Injection library that promotes best practice to steer developers towards the pit of success.
https://simpleinjector.org
MIT License
1.22k stars 152 forks source link

Disposable component not disposed of after Verify() #910

Closed PulsarFX closed 3 years ago

PulsarFX commented 3 years ago

I have a disposable component, which I register as transient. To suppress the warning in Verify() I add

 container.GetRegistration(typeof(ModuleEditorViewModel)).Registration
          .SuppressDiagnosticWarning(DiagnosticType.DisposableTransientComponent, 
                                    "disposed of by MainWindowViewModel.CloseEditor");

After Verify() has run, the component is still alive and is not disposed of. Is this expected behavior? I'd expect that the producer (container) cleans up all its created instances after he has done using them, regardless of the lifestyle.

This happens on v5.3.0

dotnetjunkie commented 3 years ago

Transient components are never disposed by Simple Injector, hense the diagnostic warning (that you are suppressing).

PulsarFX commented 3 years ago

On the live stage I understand it, that's why we have to add the suppression. In the verify stage I have no chance to do so, am I?

dotnetjunkie commented 3 years ago

Disposing transients during verification would lead to inconsistent behavior, which can be confusing.

Can you explain why it is important for that component to be disposed after verification, considering that this only happens once during startup?

PulsarFX commented 3 years ago

I thought when verify is done, somthing like allCreatedComponents.Foreach(c=> (c as IDisposable)?.Dispose() would be fine. The system would want to start from zero, like when no verify was called.

In my case it is like this: the ctor creates a filewatcher subscription using System.Reactive.Linq:

 _watcherSubscription = _fileSystemWatcherService.FileChanged
                   .Where(change => change.FullPath.StartsWith(SourceCodeDirectory.FullName))
                   .Subscribe(_ => NotifyPropertyChanged(nameof(HasChanges)));

this subscription is not cleaned up after verify and will trigger with a wrong reference to SourceCodeDirectory. In fact it will throw NullReferenceException for .Fullname as it is null during verify, as it is runtime data. I'll have to move the registration away from the ctor.

dotnetjunkie commented 3 years ago

I'll have to move the registration away from the ctor.

To me, that seems a better solution, independently of your particular issue.

There are, however, ways to manually dispose that transient instance after verification. For instance:

var container = new Container();

container.Register<ModuleEditorViewModel>();

container.GetRegistration(typeof(ModuleEditorViewModel)).Registration
    .SuppressDiagnosticWarning(DiagnosticType.DisposableTransientComponent, 
    "disposed of by MainWindowViewModel.CloseEditor");

// Create a list for objects to dispose manually
var vms = new List<IDisposable>();

// Register an initializer for the VM that adds it to the disposable list after creation
container.RegisterInitializer<ModuleEditorViewModel>(vm => vms?.Add(vm));

// Verify the container; this created ModuleEditorViewModel and adds it to the
// list (possibly multiple times)
container.Verify();

// Dispose all created ModuleEditorViewModels
vms.ForEach(vm => vm.Dispose());

// Reset the list to null to prevent the list from staying referenced and causing
// a memory leak.
vms = null;
PulsarFX commented 3 years ago

Seems like a good solution, thanks :-)

dotnetjunkie commented 3 years ago

You're welcome. There's always a workaround ;-)