Merge pull request dotnet/corefx#36735 from Wraith2/sqlfix-envchangepool

SqlClient fix SqlEnvChangePool thread safety bug Commit migrated from dotnet/corefx@af562e8
picenka21 · Apr 11, 2019 · c8d8d27 · c8d8d27
2 parents 623c7a2 + 32daacb
commit c8d8d27
Show file tree

Hide file tree

Showing 2 changed files with 2 additions and 103 deletions.
diff --git a/src/libraries/System.Data.SqlClient/src/System/Data/SqlClient/TdsParser.cs b/src/libraries/System.Data.SqlClient/src/System/Data/SqlClient/TdsParser.cs
@@ -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;
@@ -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))
                 {

diff --git a/src/libraries/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserHelperClasses.cs b/src/libraries/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserHelperClasses.cs
@@ -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