Skip to content

Commit c3857b1

Browse files
Wraith2Wraith2
authored
Fix SqlCached buffer async read with continue edge case. (#3329)
* refine logic around switching from replay to continue for SqlCachedBuffer used in xml reads * add test and cleanup unused variable * account for xml read canonicalization in tests * reduce iterations on new test
1 parent 46b88bf commit c3857b1

File tree

2 files changed

+207
-9
lines changed

2 files changed

+207
-9
lines changed

‎src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCachedBuffer.cs

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,15 @@ internal static TdsOperationStatus TryCreate(SqlMetaDataPriv metadata, TdsParser
3939
{
4040
buffer = null;
4141

42-
(bool isAvailable, bool isStarting, bool isContinuing) = stateObj.GetSnapshotStatuses();
42+
(bool isAvailable, bool isStarting, _) = stateObj.GetSnapshotStatuses();
4343

4444
List<byte[]> cachedBytes = null;
4545
if (isAvailable)
4646
{
4747
cachedBytes = stateObj.TryTakeSnapshotStorage() as List<byte[]>;
48-
if (cachedBytes != null && !isStarting && !isContinuing)
48+
if (isStarting)
4949
{
50-
stateObj.SetSnapshotStorage(null);
50+
cachedBytes = null;
5151
}
5252
}
5353

@@ -56,15 +56,13 @@ internal static TdsOperationStatus TryCreate(SqlMetaDataPriv metadata, TdsParser
5656
cachedBytes = new List<byte[]>();
5757
}
5858

59-
6059
// the very first length is already read.
6160
TdsOperationStatus result = parser.TryPlpBytesLeft(stateObj, out ulong plplength);
6261
if (result != TdsOperationStatus.Done)
6362
{
6463
return result;
6564
}
6665

67-
6866
// For now we only handle Plp data from the parser directly.
6967
Debug.Assert(metadata.metaType.IsPlp, "SqlCachedBuffer call on a non-plp data");
7068
do
@@ -105,10 +103,7 @@ internal static TdsOperationStatus TryCreate(SqlMetaDataPriv metadata, TdsParser
105103

106104
if (returnAfterAdd)
107105
{
108-
if (isStarting || isContinuing)
109-
{
110-
stateObj.SetSnapshotStorage(cachedBytes);
111-
}
106+
stateObj.SetSnapshotStorage(cachedBytes);
112107
return result;
113108
}
114109

‎src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,209 @@ public static async Task<int> CanReadEmployeesTableCompletelyWithNullImageAsync(
400400
return counter;
401401
}
402402

403+
[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
404+
public static async Task<int> CanReadXmlData()
405+
{
406+
const string xml = @"<catalog id1=""00000000-0000-0000-0000-000000000000"" id2=""00000000-0000-0000-0000-000000000000"" id3=""00000000-0000-0000-0000-000000000000"" id4=""00000000-0000-0000-0000-000000000000"" id5=""00000000-0000-0000-0000-000000000000"">
407+
<book id=""bk101"">
408+
<author>Gambardella, Matthew</author>
409+
<title>XML Developer's Guide</title>
410+
<genre>Computer</genre>
411+
<price>44.95</price>
412+
<publish_date>2000-10-01</publish_date>
413+
<description>An in-depth look at creating applications
414+
with XML.</description>
415+
</book>
416+
<book id=""bk102"">
417+
<author>Ralls, Kim</author>
418+
<title>Midnight Rain</title>
419+
<genre>Fantasy</genre>
420+
<price>5.95</price>
421+
<publish_date>2000-12-16</publish_date>
422+
<description>A former architect battles corporate zombies,
423+
an evil sorceress, and her own childhood to become queen
424+
of the world.</description>
425+
</book>
426+
<book id=""bk103"">
427+
<author>Corets, Eva</author>
428+
<title>Maeve Ascendant</title>
429+
<genre>Fantasy</genre>
430+
<price>5.95</price>
431+
<publish_date>2000-11-17</publish_date>
432+
<description>After the collapse of a nanotechnology
433+
society in England, the young survivors lay the
434+
foundation for a new society.</description>
435+
</book>
436+
<book id=""bk104"">
437+
<author>Corets, Eva</author>
438+
<title>Oberon's Legacy</title>
439+
<genre>Fantasy</genre>
440+
<price>5.95</price>
441+
<publish_date>2001-03-10</publish_date>
442+
<description>In post-apocalypse England, the mysterious
443+
agent known only as Oberon helps to create a new life
444+
for the inhabitants of London. Sequel to Maeve
445+
Ascendant.</description>
446+
</book>
447+
<book id=""bk105"">
448+
<author>Corets, Eva</author>
449+
<title>The Sundered Grail</title>
450+
<genre>Fantasy</genre>
451+
<price>5.95</price>
452+
<publish_date>2001-09-10</publish_date>
453+
<description>The two daughters of Maeve, half-sisters,
454+
battle one another for control of England. Sequel to
455+
Oberon's Legacy.</description>
456+
</book>
457+
<book id=""bk106"">
458+
<author>Randall, Cynthia</author>
459+
<title>Lover Birds</title>
460+
<genre>Romance</genre>
461+
<price>4.95</price>
462+
<publish_date>2000-09-02</publish_date>
463+
<description>When Carla meets Paul at an ornithology
464+
conference, tempers fly as feathers get ruffled.</description>
465+
</book>
466+
<book id=""bk107"">
467+
<author>Thurman, Paula</author>
468+
<title>Splish Splash</title>
469+
<genre>Romance</genre>
470+
<price>4.95</price>
471+
<publish_date>2000-11-02</publish_date>
472+
<description>A deep sea diver finds true love twenty
473+
thousand leagues beneath the sea.</description>
474+
</book>
475+
<book id=""bk108"">
476+
<author>Knorr, Stefan</author>
477+
<title>Creepy Crawlies</title>
478+
<genre>Horror</genre>
479+
<price>4.95</price>
480+
<publish_date>2000-12-06</publish_date>
481+
<description>An anthology of horror stories about roaches,
482+
centipedes, scorpions and other insects.</description>
483+
</book>
484+
<book id=""bk109"">
485+
<author>Kress, Peter</author>
486+
<title>Paradox Lost</title>
487+
<genre>Science Fiction</genre>
488+
<price>6.95</price>
489+
<publish_date>2000-11-02</publish_date>
490+
<description>After an inadvertant trip through a Heisenberg
491+
Uncertainty Device, James Salway discovers the problems
492+
of being quantum.</description>
493+
</book>
494+
<book id=""bk110"">
495+
<author>O'Brien, Tim</author>
496+
<title>Microsoft .NET: The Programming Bible</title>
497+
<genre>Computer</genre>
498+
<price>36.95</price>
499+
<publish_date>2000-12-09</publish_date>
500+
<description>Microsoft's .NET initiative is explored in
501+
detail in this deep programmer's reference.</description>
502+
</book>
503+
<book id=""bk111"">
504+
<author>O'Brien, Tim</author>
505+
<title>MSXML3: A Comprehensive Guide</title>
506+
<genre>Computer</genre>
507+
<price>36.95</price>
508+
<publish_date>2000-12-01</publish_date>
509+
<description>The Microsoft MSXML3 parser is covered in
510+
detail, with attention to XML DOM interfaces, XSLT processing,
511+
SAX and more.</description>
512+
</book>
513+
<book id=""bk112"">
514+
<author>Galos, Mike</author>
515+
<title>Visual Studio 7: A Comprehensive Guide</title>
516+
<genre>Computer</genre>
517+
<price>49.95</price>
518+
<publish_date>2001-04-16</publish_date>
519+
<description>Microsoft Visual Studio 7 is explored in depth,
520+
looking at how Visual Basic, Visual C++, C#, and ASP+ are
521+
integrated into a comprehensive development
522+
environment.</description>
523+
</book>
524+
</catalog>";
525+
526+
string tableName = DataTestUtility.GenerateObjectName();
527+
528+
SqlConnectionStringBuilder builder = new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString);
529+
builder.PersistSecurityInfo = true;
530+
builder.PacketSize = 3096; // should reproduce failure that this tests for in <100 reads
531+
532+
using (var connection = new SqlConnection(builder.ToString()))
533+
{
534+
await connection.OpenAsync();
535+
536+
try
537+
{
538+
// setup
539+
using (var dropCommand = connection.CreateCommand())
540+
{
541+
dropCommand.CommandText = $"DROP TABLE IF EXISTS [{tableName}]";
542+
dropCommand.ExecuteNonQuery();
543+
}
544+
545+
using (var createCommand = connection.CreateCommand())
546+
{
547+
createCommand.CommandText = $"CREATE TABLE [{tableName}] (Id int PRIMARY KEY, Data xml NOT NULL)";
548+
createCommand.ExecuteNonQuery();
549+
}
550+
551+
for (var i = 0; i < 100; i++)
552+
{
553+
using (var insertCommand = connection.CreateCommand())
554+
{
555+
insertCommand.CommandText = $"INSERT INTO [{tableName}] (Id, Data) VALUES (@id, @data)";
556+
insertCommand.Parameters.AddWithValue("@id", i);
557+
insertCommand.Parameters.AddWithValue("@data", xml);
558+
insertCommand.ExecuteNonQuery();
559+
}
560+
}
561+
562+
// execute
563+
int id = 0;
564+
565+
using (var command = connection.CreateCommand())
566+
{
567+
command.CommandText = $"SELECT Data FROM [{tableName}] ORDER BY Id";
568+
using (var reader = await command.ExecuteReaderAsync())
569+
{
570+
string expectedResult = null;
571+
while (await reader.ReadAsync())
572+
{
573+
var result = reader.GetString(0);
574+
if (expectedResult == null)
575+
{
576+
expectedResult = result;
577+
}
578+
else
579+
{
580+
Assert.Equal(expectedResult, result);
581+
}
582+
id++;
583+
}
584+
}
585+
}
586+
return id;
587+
588+
}
589+
finally
590+
{
591+
try
592+
{
593+
using (var dropCommand = connection.CreateCommand())
594+
{
595+
dropCommand.CommandText = $"DROP TABLE IF EXISTS [{tableName}]";
596+
dropCommand.ExecuteNonQuery();
597+
}
598+
}
599+
catch
600+
{
601+
}
602+
}
603+
}
604+
}
605+
403606
// Synapse: Cannot find data type 'rowversion'.
404607
[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureSynapse))]
405608
public static void CheckLegacyNullRowVersionIsEmptyArray()

0 commit comments

Comments
 (0)