Fix possible deadlock while asynchronously reading values from reader (#6202)

vonzshik · web-flow · commit 7bbf43a67958 · 2025-09-10T11:32:51.000Z
Fixes #6190
diff --git a/src/Npgsql/Internal/Converters/AsyncHelpers.cs b/src/Npgsql/Internal/Converters/AsyncHelpers.cs
@@ -37,9 +37,17 @@ public abstract class CompletionSource
 
     public sealed class CompletionSource<T> : CompletionSource
     {
-        AsyncValueTaskMethodBuilder<T> _amb = AsyncValueTaskMethodBuilder<T>.Create();
+        AsyncValueTaskMethodBuilder<T> _amb;
 
-        public ValueTask<T> Task => _amb.Task;
+        public ValueTask<T> Task { get; }
+
+        public CompletionSource()
+        {
+            _amb = AsyncValueTaskMethodBuilder<T>.Create();
+            // AsyncValueTaskMethodBuilder's Task and SetResult aren't thread safe in regard to each other
+            // Which is why we access it prematurely
+            Task = _amb.Task;
+        }
 
         public void SetResult(T value)
             => _amb.SetResult(value);
@@ -50,9 +58,17 @@ public override void SetException(Exception exception)
 
     public sealed class PoolingCompletionSource<T> : CompletionSource
     {
-        PoolingAsyncValueTaskMethodBuilder<T> _amb = PoolingAsyncValueTaskMethodBuilder<T>.Create();
+        PoolingAsyncValueTaskMethodBuilder<T> _amb;
 
-        public ValueTask<T> Task => _amb.Task;
+        public ValueTask<T> Task { get; }
+
+        public PoolingCompletionSource()
+        {
+            _amb = PoolingAsyncValueTaskMethodBuilder<T>.Create();
+            // PoolingAsyncValueTaskMethodBuilder's Task and SetResult aren't thread safe in regard to each other
+            // Which is why we access it prematurely
+            Task = _amb.Task;
+        }
 
         public void SetResult(T value)
             => _amb.SetResult(value);
@@ -90,7 +106,7 @@ public CompletionSourceContinuation(object handle, delegate*<Task, CompletionSou
         if (task.IsCompletedSuccessfully)
             return new(new T?(task.Result));
 
-        // Otherwise we do one additional allocation, this allow us to share state machine codegen for all Ts.
+        // Otherwise we do one additional allocation, this allows us to share state machine codegen for all Ts.
         var source = new PoolingCompletionSource<T?>();
         OnCompletedWithSource(task.AsTask(), source, new(instance, &UnboxAndComplete));
         return source.Task;
@@ -111,7 +127,7 @@ public static unsafe ValueTask<T> ReadAsObjectAsyncAsT<T>(this PgConverter<T> in
         if (task.IsCompletedSuccessfully)
             return new((T)task.Result);
 
-        // Otherwise we do one additional allocation, this allow us to share state machine codegen for all Ts.
+        // Otherwise we do one additional allocation, this allows us to share state machine codegen for all Ts.
         var source = new PoolingCompletionSource<T>();
         OnCompletedWithSource(task.AsTask(), source, new(instance, &UnboxAndComplete));
         return source.Task;
