Fix AsyncLocal leak in SystemTransactionContext

[pecanw]

Fixes #1594.

[fredericDelaporte]

We support only 2.0. And we have defined a NETFX variable you can use instead, of course with opposite semantic.

[pecanw]

Changed to use NETFX.

[fredericDelaporte]

Since it uses system transaction, the fixture must be disabled if the environment does not support them. Add:

protected override bool AppliesTo(ISessionFactoryImplementor factory) =>
	factory.ConnectionProvider.Driver.SupportsSystemTransactions;

(Since currently this is false for all .Net Core / .Standard drivers, the private field fix is in fact not really needed, but it does not harm to have it.)

[pecanw]

Done, thanks for describing.

[hazzik]

As I understand when one talks about memory leaks what they mean is that the objects survive the GC. The test does not consider any GC at all at the moment.

We have discovered previously (for the other matter) that AsyncLocal<> has heavy memory footprint, but in majority of the cases the memory becomes reclaimed after the GC.

[pecanw]

As for the @hazzik's comment: in this case every time when AsyncLocal<bool> _bypassLock is assigned a value the current ExecutionContext._localValues dictionary gets copied into a new one with a new / updated value of that _bypassLock object instance. The GC can collect the original dictionary, but the new copy (with the additional item in case of the first assignment to the _bypassLock instance) is referenced in the ExecutionContext and thus cannot be released by the GC.

[fredericDelaporte]

This is true in both case, when the async local is static or not. But when it is not static, the current context dictionary references all past instances which have been created and have seen their value affected on the same thread, and grows endlessly till thread end.

The test does not consider any GC at all at the moment.

GC can not change anything in the test, since the multiple created asynclocal stay all at any point in time referenced in a dictionary, itself referenced in a instance referenced as a thread static (without weak references anywhere).

[pecanw]

I have a question not related to this problem - I have the latest version of Resharper installed on my PC and it auto-updates the NHibernate.sln.DotSettings. I have unstaged it not to include it in the commit, but it is quite annoying to keep it it still in mind. May be other contributors have similar problem. Can you tell me if I

• should keep it it is
• include the change in the pull request
• create a new branch + pull request for this specific think
• just add a new item to NH backlog to let someone do it?

[fredericDelaporte]

Yes we know, and there is the same trouble with Rider. It does that at almost each update, which is quite a nuisance. Sometimes we let it get updated with an unrelated PR. (May be done in #1598.)

[hazzik]

GC can not change anything in the test

It can demonstrate and prove that the claims are true.

[fredericDelaporte]

For the record, async re-generation was forgotten is this PR and has been fixed with #1608. (See contributing if you need more information on async generation.)