Revert change that made SqlDataReader.ReadAsync() non-blocking

[cheenamalhotra]

This PR reverts dotnet/corefx#26595 and #490 (attempted to fix MARS TDS header issues introduced with dotnet/corefx#26595) with additional tests.

This is an alternate solution to fix #542 but due to #544 and #545 failing intermittently on Unix w. Managed SNI, it doesn't seem an appropriate fix either.

This revert is likely to hit performance by re-introducing old behavior of SqlDataReader.ReadAsync() method to run synchronously, and will block the calling thread until data is fed from SQL Server. But given the side-effects of the original change, reverting seems most appropriate at this point and a proper investigation would be needed to be done for original performance issue.

[ErikEJ]

@cheenamalhotra still planning a 2.0 preview3 release soon?

[kornelpal]

After changing the target framework to .NET Framework 4.8, we started to encounter random MARS errors even though we are not using MARS: The incoming tabular data stream (TDS) protocol stream is incorrect. The MARS TDS header contained errors.

Unfortunately there is extremely limited amount of information available, and took us several days to figure out what is happening. The comment of @David-Engel on issue #85 was most helpful to us.

Although most official documentation is not listing the change, I remembered that .NET Framework 4.8 Release Notes mentions that SqlDataReader.ReadAsync() now runs asynchronously that was the next useful pointer.

Unfortunately the "Switch.Microsoft.Data.SqlClient.MakeReadAsyncBlocking" AppContext switch did not work on .NET Framework 4.8, and I could not find any relevant documentation about it either. While the searchable .NET Framework Reference Source and the referencesource repository do not include the relevant LocalAppContextSwitches class, luckily the downloadable version contains the missing LocalAppContextSwitches.cs file that helped figuring out the correct workaround:
AppContext.SetSwitch("Switch.System.Data.SqlClient.MakeReadAsyncBlocking", true);

Note that AppContextDefaultValues.Defaults.cs from the downloadable Reference Source shows that targeting .NET Framework 4.8 changes the default value from true to false.

Since enabling the AppContext switch, we have not seen any more MARS errors, but apparently we are using an undocumented switch. Although we haven't tried those, other possible workarounds are targeting a version earlier than .NET Framework 4.8 or not using SqlDataReader.ReadAsync().

@cheenamalhotra and @David-Engel, I believe that .NET Framework 4.8 is also affected by this issue (#547). Could you please share whether you have any plans of backporting this fix/revert to .NET Framework 4.8?

[cheenamalhotra]

@kornelpal

Do you have a repro code available that we can try to reproduce what you observed?
Our test case (AsyncCancelledConnectionsTest) that was added specifically for this issue in .NET Core does not reproduce the error in .NET Framework 4.8.

[kornelpal]

I could easily reproduce the issue with your test code. I run it in a standalone console application, but I made trivial changes only to remove dependency on the unit testing framework.

Your AppDomain has to target .NET Framework 4.8, not just run on .NET Framework 4.8 (or the .dll target it) to have the switch disabled by default. AppContext.TargetFrameworkName returns ".NETFramework,Version=v4.8" in this case. Your unit test framework might not target .NET Framework 4.8.

Alternatively, you can disable the switch when running on .NET Framework 4.8, but not targeting it:
AppContext.SetSwitch("Switch.System.Data.SqlClient.MakeReadAsyncBlocking", false);

[cheenamalhotra]

Hi @kornelpal

Yes I can reproduce issue with System.Data.SqlClient (default behavior, no custom switches).
I was testing initially with Microsoft.Data.SqlClient that does not reproduce this issue by default, but only when setting below switch:

AppContext.SetSwitch("Switch.Microsoft.Data.SqlClient.MakeReadAsyncBlocking", false);

You can find documentation of App Context switches for Microsoft.Data.SqlClient here: https://docs.microsoft.com/en-us/sql/connect/ado-net/appcontext-switches?view=sql-server-ver15. I agree the System.Data.SqlClient switches are not documented well, we'll update our documentation to provide notes for users for transferred switches.

In regards to System.Data.SqlClient, since the product is no longer actively maintained but only receives high security issue updates, I'm afraid the default behavior of S.D.S is likely not going to change. I'd recommend you to transition to Microsoft.Data.SqlClient as we actively maintain and deliver new releases and bug fixes here now.