Prevent multiple invocations to dispose `ICompletableSynchronizedStorageSession` implementations

[ramonsmits]

Describe the suggested improvement

Is your improvement related to a problem? Please describe.

In LoadHandlersConnector the resolved ICompletableSynchronizedStorageSession instance gets dispose multiple times:

• 1x in LoadHandlersConnector.Invoke due to the using scope
• 1x in MainPipelineExecutor due to its await using on the Dependency Injection child scope.

On top of that some implementations also invoke Dispose / DisposeAsync in ICompletableSynchronizedStorageSession.CompleteAsync meaning, disposing of these sub resources can be invoked 3 times.

Implementations SHOULD have an idempotent Dispose:

• Microsoft's .NET documentation on IDisposable.Dispose() explicitly states: "The Dispose method is designed to be called more than once without throwing an exception." Source: https://learn.microsoft.com/en-us/dotnet/api/system.idisposable.dispose
• The Framework Design Guidelines (authored by Krzysztof Cwalina and Brad Abrams, Microsoft architects) states that Dispose should be idempotent.

Although implementation SHOULD be idempotent this isn't a guarantee and can result in issues like:

• https://github.com/Particular/NServiceBus.Persistence.Sql/issues/1235

As Core is in full control of the stack/scope ICompletableSynchronizedStorageSession.Dispose should only be needed to be invoked once.

Describe the suggested solution

Some options:

• Remove the using in LoadHandlersConnector:
  • Not a good option as the using statement here exists to control the transaction scope, not specifically the lifetime of any resources. Removing the using would result in:
    • Commit/Save in CompleteAsync
    • Rollback to happen during disposing of DI child scope which would be strange as after LoadHandlersConnector.Invoke the transaction should be considered to be done. If its allowed to rollback/dispose much later, then why is the transaction started here but not much earlier.
• Not resolve ICompletableSynchronizedStorageSession via DI so that it is not disposed during disposal of the DI child scope.
  • A factory could be registered in DI that returns instances which lifetime to be managed outside of DI
• A new child scope via IServiceProvider.CreateAsyncScope that only is used to resolve ICompletableSynchronizedStorageSession
  • Pro: Nice to use DI for disposing
  • Con: This would be a new DI child scope, not sharing any resources with the scope created in MainPipelineExecutor. As ISqlStorageSession can also be resolved in message handlers via constructor injection](https://docs.particular.net/persistence/sql/accessing-data) this is problematic
    • The new child scope needs to be used to resolve IHandleMessages in order to share the same instance but doing to would result in not sharing the resources with the scope created in MainPipelineExecutor.

The above indicates that it is fairly complex to prevent Dispose to be invoked only once. This is partially due to the design of ICompletableSynchronizedStorageSession and due to how .NET uses using to manage transactions. Instead of using IAsyncDisposable / IDisposable the interface could also use explicit methods like Commit and Rollback in combination with a finally but this would deviate from the transaction pattern which is very common in .net.

Additional Context

No response