Misleading ADP_OpenReaderExists exception message on MARS-disabled Sql Connection when incorrectly doing parallel requests

[galkinvv]

I've discovered the issue using ordinary .net4.5 on Windows but further investigating shows that it is related to code that now is in the .net Core

While simultaneousely (incrorrcetly) using single SqlConnection with Multiple Active Result Sets (MARS) disabled an exception with text

<data name="ADP_OpenReaderExists" xml:space="preserve">
<value>There is already an open DataReader associated with this Command which must be closed first.</value>
</data>

Note that text mentions same "Command" but in reality the resource taht is wrongly shared is not command but connection.

The exception is raised from https://github.com/dotnet/corefx/blob/cf19c22be88b5ea31b5e2ea69ab3632f930b1e26/src/System.Data.SqlClient/src/System/Data/SqlClient/SqlInternalConnectionTds.cs#L587

The comment

// if MARS is on, then a datareader associated with the command exists
// or if MARS is off, then a datareader exists

above raising the exception is absolutely correct - if MARS is on the exception text is correct - problem is in using multiple readers with single command. But if mars is off the checks above checking for any active datareaders for the same connection, not only for datareaders for the same command.

So the problem is that information about what scope (Connection or Command) was checked is not passed to exception cinstructor and it incorrectly always says that there is another reader associated with the command

[galkinvv]

I think that problem raises quite commonly: see comments below this post: (not the post itself!) https://blogs.msdn.microsoft.com/spike/2009/08/20/there-is-already-an-open-datareader-associated-with-this-command-which-must-be-closed-first-explained/

[joshfree]

@saurabh500 @corivera

[divega]

As recently announced in the .NET Blog, focus on new SqlClient features an improvements is moving to the new Microsoft.Data.SqlClient package. For this reason, we are moving this issue to the new repo at https://github.com/dotnet/SqlClient. We will still use https://github.com/dotnet/corefx to track issues on other providers like System.Data.Odbc and System.Data.OleDB, and general ADO.NET and .NET data access issues.

[David-Engel]

This sounds like a relatively simple fix. In https://github.com/dotnet/corefx/blob/cf19c22be88b5ea31b5e2ea69ab3632f930b1e26/src/System.Data.SqlClient/src/System/Data/SqlClient/SqlInternalConnectionTds.cs#L587, we can throw an exception with a different message if mars is on versus off.

[yukiwongky]

@galkinvv Attached is the driver with the fix in PR #250. Can you verify it works?
Microsoft.Data.SqlClient.1.1.0-dev.zip

[galkinvv]

Thanks for the fix! However unfortunately now I doesn't have corresponding environment, so I can't actually test it.
Though, I reviewed the PR code:fix looks fine and the tests are looking similar to my older code that was causing this issue.

So I think that this bug should be closed after merging PR.

[yukiwongky]

Closing this issue since PR #250 is merged.