Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

SqlClient fix SqlEnvChangePool thread safety bug #36735

Merged
merged 4 commits into from
Apr 11, 2019
Merged

SqlClient fix SqlEnvChangePool thread safety bug #36735

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b9bd354
change SqlEnvChangePool to be simpler and safe
Wraith2 Apr 9, 2019
7388539
address feedback
Wraith2 Apr 9, 2019
f27d235
spelling feedback
Wraith2 Apr 9, 2019
538cf16
remove pool
Wraith2 Apr 9, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1937,7 +1937,7 @@ internal bool TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataRead
}
SqlEnvChange head = env;
env = env.Next;
SqlEnvChangePool.Release(head);
head.Clear();
head = null;
}
break;
Expand Down Expand Up @@ -2187,7 +2187,7 @@ private bool TryProcessEnvChange(int tokenLength, TdsParserStateObject stateObj,

while (tokenLength > processedLength)
{
SqlEnvChange env = SqlEnvChangePool.Allocate();
var env = new SqlEnvChange();

if (!stateObj.TryReadByte(out env.type))
{
Expand Down
101 changes: 0 additions & 101 deletions src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserHelperClasses.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,107 +298,6 @@ internal void Clear()
}
}

internal static class SqlEnvChangePool
{
private static int _count;
private static int _capacity;
private static SqlEnvChange[] _items;
#if DEBUG
private static string[] _stacks;
#endif

static SqlEnvChangePool()
{
_capacity = 10;
_items = new SqlEnvChange[_capacity];
_count = 1;
#if DEBUG
_stacks = new string[_capacity];
#endif
}

internal static SqlEnvChange Allocate()
{
SqlEnvChange retval = null;
lock (_items)
{
while (_count > 0 && retval is null)
{
int count = _count; // copy the count we think we have
int newCount = count - 1; // work out the new value we want
// exchange _count for newCount only if _count is the same as our cached copy
if (count == Interlocked.CompareExchange(ref _count, newCount, count))
{
// count is now the previous value, we're the only thread that has it, so count-1 is safe to access
Interlocked.Exchange(ref retval, _items[count - 1]);
}
else
{
// otherwise the count wasn't what we expected, spin the while and try again.
}
}
}
if (retval is null)
{
retval = new SqlEnvChange();
}
return retval;
}

internal static void Release(SqlEnvChange item, [Runtime.CompilerServices.CallerMemberName] string caller = null)
{
if (item is null)
{
Debug.Fail("attenpting to release null packet");
return;
}
item.Clear();
{
#if DEBUG
if (_count > 0)
{
for (int index = 0; index < _count; index += 1)
{
if (object.ReferenceEquals(item, _items[index]))
{
Debug.Assert(false,$"releasing an item which already exists in the pool, count={_count}, index={index}, caller={caller}, released at={_stacks[index]}");
}
}
}
#endif
int tries = 0;

while (!(item is null) && tries < 3 && _count < _capacity)
{
int count = _count;
int newCount = count + 1;
if (count == Interlocked.CompareExchange(ref _count, newCount, count))
{
_items[count] = item;
#if DEBUG
_stacks[count] = caller;
#endif
item = null;
}
else
{
tries += 1;
}
}
}
}

internal static int Count => _count;

internal static int Capacity => _capacity;

internal static void Clear()
{
Array.Clear(_items, 0, _capacity);
_count = 0;
}
}

internal sealed class SqlLogin
{
internal int timeout; // login timeout
Expand Down