abort connection at sqltransaction

[yukiwongky]

Fixes #291

[Wraith2]

You could use a catch when clause,
catch (SqlException ex) when (ex.InnerException is Win32Exception win32ex && win32ex.NativeErrorCode == TdsEnums.SNI_WAIT_TIMEOUT) but it's just a style choice so not important if you want to keep it as is.

Test(s)?

The removal of the DoomConnectionNoReuse calls will now allow the transaction to be repooled, is that intented? If so there seem to be no callers to the method anymore so it can be removed. If aborting the connection causes it not to be re-pooled (which seem to be the suggestion from the new comment) why was this function needed originally?

[David-Engel]

The removal of the DoomConnectionNoReuse calls will now allow the transaction to be repooled, is that intented?

It looks like dooming the connection also causes it to not be returned to the pool. CanBePooled references: (!_connectionIsDoomed && !_cannotBePooled && !_owningObject.IsAlive).

If so there seem to be no callers to the method anymore so it can be removed.

Agreed. That was added as part of the first attempted fix for 130 and should be removed here.

If aborting the connection causes it not to be re-pooled (which seem to be the suggestion from the new comment) why was this function needed originally?

Overly cautious trying to fix the issue originally since we could not repro it.

[yukiwongky]

Test(s)?

I couldn't write a test for it since it's difficult to reproduce a commit timeout consistently.. The only way I could reproduce the timeout for consistently is by changing the connection timeout to 1ms in the driver. This can not be done in a client application or test because minimum timeout that can be set in the connection string is 1 second.

The removal of the DoomConnectionNoReuse calls will now allow the transaction to be repooled, is that intented?

I removed this part because by the time DoomConnectionNoReuse is called, the _innerConnection is already detached from the innerTransaction, and _innerConnection is null at that time so dooming it isn't useful.

If so there seem to be no callers to the method anymore so it can be removed.

Yes that's right. I will remove the function in the next change.

[Wraith2]

This can not be done in a client application or test because minimum timeout that can be set in the connection string is 1 second.

There are a number of places in the tests that use private reflection to alter internal data to help cause specific scenarios. If the test would be repeatable with only the capacity to alter that timeout it might be worth doing the same here.