Remove session finalizer

[bahusoid]

Turns out exception is possible from session Finalizer (regression made by #1984):

Unhandled Exception: System.NullReferenceException: Object reference not set to an instance of an object.
   at NHibernate.Impl.AbstractSessionImpl.BeginContext() in /home/runner/work/nhibernate-core/nhibernate-core/src/NHibernate/Impl/AbstractSessionImpl.cs:line 419
   at NHibernate.Impl.SessionImpl.Dispose(Boolean isDisposing) in /home/runner/work/nhibernate-core/nhibernate-core/src/NHibernate/Impl/SessionImpl.cs:line 1531
   at NHibernate.Impl.SessionImpl.Finalize() in /home/runner/work/nhibernate-core/nhibernate-core/src/NHibernate/Impl/SessionImpl.cs:line 1491

Suggested fix: Let's simply drop finalizers. They shouldn't be created in a first place - we have no unmanaged resources inside session. So we add additional pressure on finalizer thread for nothing

[fredericDelaporte]

I would either get rid of Dispose(bool isDisposing) in this PR too, or only remove the using (BeginContext()) we have in Dispose(bool isDisposing), which is undue. (Dispose() already handles it, and there is no reason to handle the context when finalizing, instead it must not be handled during finalization.)

With this current PR state, we end up with a partial dispose pattern implementation, which can be misleading for future changes. It looks like it handles finalization since it still has that disposing flag, but it does not.

I am for removing the finalizer and the isDisposing overload, at least for next minor. In this release branch, just removing the undue BeginContext will be enough, if not wiling to also remove the isDisposing overload.

[bahusoid]

we end up with a partial dispose pattern implementation, which can be misleading for future changes

I find confusing that this pattern is implemented but no unmanaged resources are handled by it. So I've added explicit comment about it. So IMHO no reasons to keep finalizers just to implement properly not needed pattern.

I would either get rid of Dispose(bool isDisposing) in this PR to

Yeah I didn't remove parameter to minimize changes as it's called from 2 places (from Dispose and CloseSessionFromSystemTransaction) and it's not straightforward change. Is it actually correct that CloseSessionFromSystemTransaction doesn't call full Dispose() method? It looks fishy to me...

it would be safer to just avoid calling BeginContext while finalizing.

Logger also can cause troubles - some loggers might be already disposed. Really I see zero reasons to keep finalizers. Let's drop them.

[fredericDelaporte]

Is it actually correct that CloseSessionFromSystemTransaction doesn't call full Dispose() method? It looks fishy to me...

Dispose has already been called when CloseSessionFromSystemTransaction is called... All this is really fishy, but that is the legacy "flush on system transaction completion" feature which requires that.
Here is the whole pattern:

• Open a transaction scope.
• Open a session
• Dispose the session. Actually the session disposal is delayed, the transaction context is flagged for disposing the session later.
• Complete the transaction scope. The 2PC will then handle the session actual disposal by calling CloseSessionFromSystemTransaction.

[fredericDelaporte]

Ok, forget it, I have re-read the thing, you are right.