Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NullReferenceException when streaming XML asynchronously from DB #1903

Closed
NickKerensky opened this issue Jan 25, 2023 · 10 comments · Fixed by #1906
Closed

NullReferenceException when streaming XML asynchronously from DB #1903

NickKerensky opened this issue Jan 25, 2023 · 10 comments · Fixed by #1906
Labels
💥 Regression Regression introduced from earlier PRs

Comments

@NickKerensky
Copy link

In our environment we frequently stream XML columns from the database using a pattern similar to the one in the repro test case.
After upgrading to Microsoft.Data.SqlClient 5.1.0, we're getting NullReferenceExceptions for code that was previously working.

Exception message: An exception of type 'System.NullReferenceException' occurred in System.Private.CoreLib.dll but was not handled in user code: 'Object reference not set to an instance of an object.'
Stack trace:
   at Microsoft.Data.SqlClient.SqlDataReader.SqlDataReaderAsyncCallContext`2.DisposeCore()
   at Microsoft.Data.SqlClient.AAsyncBaseCallContext`2.ClearCore()
   at Microsoft.Data.SqlClient.AAsyncBaseCallContext`2.Dispose()
   at Microsoft.Data.SqlClient.SqlDataReader.CompleteAsyncCall[T](Task`1 task, SqlDataReaderBaseAsyncCallContext`1 context)
   at Microsoft.Data.SqlClient.SqlDataReader.InvokeAsyncCall[T](SqlDataReaderBaseAsyncCallContext`1 context)
--- End of stack trace from previous location ---
   at System.Runtime.CompilerServices.ConfiguredValueTaskAwaitable`1.ConfiguredValueTaskAwaiter.GetResult()
   at System.Xml.XmlTextReaderImpl.<ReadDataAsync>d__504.MoveNext()
   at System.Xml.XmlTextReaderImpl.<FinishInitTextReaderAsync>d__477.MoveNext()
   at System.Xml.AsyncHelper.<CallBoolTaskFuncWhenFinishCoreAsync>d__11`1.MoveNext()
   at System.Xml.Linq.XDocument.<LoadAsyncInternal>d__25.MoveNext()
   at Program.<<Main>$>d__0.MoveNext() in C:\Users\user\source\repos\testsql\Program.cs:line 43
   at Program.<<Main>$>d__0.MoveNext() in C:\Users\user\source\repos\testsql\Program.cs:line 33
   at Program.<<Main>$>d__0.MoveNext() in C:\Users\user\source\repos\testsql\Program.cs:line 33
   at Program.<<Main>$>d__0.MoveNext() in C:\Users\user\source\repos\testsql\Program.cs:line 33

To reproduce

This is the simplest test case I could build for the error we're seeing. It's the same:

  • on-prem or Azure SQL
  • Windows or Linux
  • .NET 6.0 or 7.0

If you downgrade SqlClient to 5.0.1 you will get the expected result; otherwise there's a NullReferenceException. Applying one of the HACK: lines below will usually provide the expected result with 5.1.0, but not always. (e.g. We've seen process hangs after using the IsDBNullAsync workaround.)

using System;
using System.Data;
using System.Xml;
using System.Xml.Linq;
using Microsoft.Data.SqlClient;

// configurations tested with identical results:
// -Azure SQL with AAD MSI on Windows (IIS/ASP.NET 6.0)
// -Azure SQL with AAD Interactive on Windows (.NET 7.0)
// -Azure SQL with SQL username/password on WSL2 Ubuntu Jammy (.NET 7.0)
// -on-prem SQL 2017 CU 31 with Trusted Connection on Windows (.NET 7.0)
const string ConnectionString = "<provide your own>";

/*
 * The code listed below will throw an NullReferenceException with SqlClient 5.1.0.
 * If you change the SqlClient package to 5.0.1, or apply one of the three HACK lines
 * of code below you will get the expected output.
 */
await using var con = new SqlConnection(ConnectionString);
await using var command = new SqlCommand(
    // original case was selecting a single row from a table with an XML NOT NULL column; same behavior observed
    "SELECT CAST('<test>This is a test string</test>' AS NVARCHAR(MAX))", 
    con
) { CommandType = CommandType.Text };

await con.OpenAsync().ConfigureAwait(false);
await using var reader = await command.ExecuteReaderAsync(
    // HACK: no NRE if you comment out SequentialAccess
    CommandBehavior.SequentialAccess
    ).ConfigureAwait(false);

var xmlOpts = new XmlReaderSettings() { Async = true };
while (await reader.ReadAsync().ConfigureAwait(false)
    // HACK: no NRE if you check for DBNull, even though we know it's not null
    //&& !await reader.IsDBNullAsync(0).ConfigureAwait(false)
    )
{
    using var textR = reader.GetTextReader(0);
    using var xmlR = XmlReader.Create(textR, xmlOpts);

    // HACK: no NRE if you load xmlR synchronously
    //var xdoc = XDocument.Load(xmlR);
    var xdoc = await XDocument.LoadAsync(xmlR, LoadOptions.None, default)
        .ConfigureAwait(false); // NRE thrown here

    // Expected: "<test>This is a test string</test>"
    Console.WriteLine(xdoc.ToString());
}

Expected behavior

Prints the test XML string to the console.

Further technical details

Microsoft.Data.SqlClient version: 5.1.0
.NET target: .NET 6.0, .NET 7.0
SQL Server version: Azure SQL, SQL Server 2017
Operating system: Windows 2019, Windows 10 21H2, Ubuntu 22.04

@lcheunglci lcheunglci added this to Needs triage in SqlClient Triage Board via automation Jan 25, 2023
@lcheunglci
Copy link
Contributor

Hi @NickKerensky, thanks for reporting this issue and providing the sample repro code snippet. It sounds like a regression that might be caused by one of our changes in the release. We'll do some investigation and get back to you soon.

@Wraith2
Copy link
Contributor

Wraith2 commented Jan 25, 2023

From the stack trace this looks like code I've been very involved with. Do you want me to have a look @lcheunglci ?

@lcheunglci
Copy link
Contributor

Thanks @Wraith2, your help would be much appreciated. :)

@Wraith2
Copy link
Contributor

Wraith2 commented Jan 25, 2023

Simple fix, just need to add a ? in the right place. #1906

@lcheunglci
Copy link
Contributor

Thanks @Wraith2!

@lcheunglci lcheunglci moved this from Needs triage to Under Investigation in SqlClient Triage Board Jan 26, 2023
@lcheunglci lcheunglci moved this from Needs triage to Under Investigation in SqlClient Triage Board Jan 26, 2023
@lcheunglci lcheunglci added 💥 Regression Regression introduced from earlier PRs and removed untriaged labels Jan 26, 2023
SqlClient Triage Board automation moved this from Under Investigation to Closed Jan 30, 2023
@udlose
Copy link

udlose commented Feb 21, 2023

@Wraith2 , @lcheunglci Anyone have an idea when a hotfix will be released? We are currently experiencing the same NRE. Is it possible to have something published before 5.2.0?

@Wraith2
Copy link
Contributor

Wraith2 commented Feb 22, 2023

No idea. you could use the artifacts from the PR if you want.

@lcheunglci
Copy link
Contributor

Hi @udlose, since MDS 5.1 is an LTS release, we would expect a hotfix release with this fix included in the near future. However, I don't have the exact date at the moment as we're in the middle of planning for the first half of the year. Once we have a concrete date, we'll let you know, but as @Wraith2 mentioned, you can use the artifacts from the PR at the moment.

@lcheunglci
Copy link
Contributor

We currently don't have a schedule for a 5.1.1 hotfix release; however, depending on the number of bugs and severity, it could happen within a month or 2.

@lcheunglci
Copy link
Contributor

@udlose the 5.1.1 hotfix release came out yesterday that include this fix .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💥 Regression Regression introduced from earlier PRs
Projects
Development

Successfully merging a pull request may close this issue.

4 participants