Exception while disposing the ioc container at the end of a scenario leaves the InternalContextManager in a incomplete state

[bollhals]

SpecFlow Version

3.9.74

Which test runner are you using?

MSTest

Test Runner Version Number

3.0.1

.NET Implementation

.NET 6.0

Project Format of the SpecFlow project

Classic project format using <PackageReference> tags

.feature.cs files are generated using

SpecFlow.Tools.MsBuild.Generation NuGet package

Test Execution Method

Visual Studio Test Explorer

SpecFlow Section in app.config or content of specflow.json

No response

Issue Description

At the end of a scenario, the context manager gets cleaned up and it will dispose the ioc container. If during that dispose, one of the singleton instances which are disposable throws an exception, it will bubble up and leave the objectContainer & instance variable set/not null. Example stacktrace

at MySingletonObject.Dispose()
at BoDi.ObjectContainer.Dispose()
at TechTalk.SpecFlow.Infrastructure.ContextManager.InternalContextManager`1.DisposeInstance()
at TechTalk.SpecFlow.Infrastructure.ContextManager.InternalContextManager`1.Cleanup()
at TechTalk.SpecFlow.Infrastructure.ContextManager.CleanupScenarioContext()
at TechTalk.SpecFlow.Infrastructure.TestExecutionEngine.OnScenarioEnd()
at TechTalk.SpecFlow.TestRunner.OnScenarioEnd()
at MyFeature.TestTearDown()

For the next scenario being executed, it will try to init the context manager, but since the instance is not null (due to the exception above) it will enter this case and trace a warning before trying again to dispose it. Due to writing the warning, the MSTestTraceListener is called and tries to get the test context, which needs to be resolved from the ioc container, which is already disposed, and then throws another exception, which brings down this test case as well.

Example stacktrace

at BoDi.ObjectContainer.AssertNotDisposed()
at BoDi.ObjectContainer.Resolve(Type typeToResolve, ResolutionList resolutionPath, String name)
at BoDi.ObjectContainer.Resolve(Type typeToResolve, String name)
at BoDi.ObjectContainer.Resolve[T](String name)
at BoDi.ObjectContainer.Resolve[T]()
at TechTalk.SpecFlow.MSTest.SpecFlowPlugin.MSTestTestContextProvider.GetTestContext()
at TechTalk.SpecFlow.MSTest.SpecFlowPlugin.MSTestTraceListener.WriteToolOutput(String message)
at TechTalk.SpecFlow.Tracing.TraceListenerHelper.WriteToolOutput(ITraceListener traceListener, String messageFormat, Object[] args)
at TechTalk.SpecFlow.Tracing.TestTracer.TraceWarning(String text)
at TechTalk.SpecFlow.Infrastructure.ContextManager.InternalContextManager`1.Init(TContext newInstance, IObjectContainer newObjectContainer)
at TechTalk.SpecFlow.Infrastructure.ContextManager.InitializeScenarioContext(ScenarioInfo scenarioInfo)
at TechTalk.SpecFlow.Infrastructure.TestExecutionEngine.OnScenarioInitialize(ScenarioInfo scenarioInfo)
at TechTalk.SpecFlow.TestRunner.OnScenarioInitialize(ScenarioInfo scenarioInfo)
at MyFeature.ScenarioInitialize(ScenarioInfo scenarioInfo)

I know throwing an exception in the dispose is an issue on our side, but the fact that it causes the next scenario to fail is something rather unexpected and confusing.

The fix I think is rather simple version of one of the following replacements for this method:

            private void DisposeInstance()
            {
                // null the variables before calling dispose in case it throws
                var container = objectContainer;
                instance = null;
                objectContainer = null;
                container?.Dispose();
            }

OR

            private void DisposeInstance()
            {
                try
                {
                    objectContainer?.Dispose();
                }
                finally
                {
                    // make sure the variables are null even if dispose throws
                    instance = null;
                    objectContainer = null;
                }
            }

Steps to Reproduce

see above

Link to Repro Project

No response

[bollhals]

I'm happy to provide a PR with the fix if you'd be ok with such a change

[bollhals]

ping on this, as this is causing regularely failed tests on our end.

[dinaRodeny]

same issue in my code