Properly Dispose SocketAsyncEventArgs' FileStreams. (dotnet/corefx#34331)
authorKurt Schelfthout <kurt.schelfthout@gmail.com>
Fri, 11 Jan 2019 15:02:37 +0000 (15:02 +0000)
committerStephen Toub <stoub@microsoft.com>
Fri, 11 Jan 2019 15:02:37 +0000 (10:02 -0500)
* Properly Dispose SocketAsyncEventArgs' FileStreams.

* Add a test.

* Make the first call in the test succeed.

Otherwise the socket might be disconnected in error by the time we get to the second call.

* Skip test on .net framework as that does not have the fix.

* Fix comment.

Commit migrated from https://github.com/dotnet/corefx/commit/9ec2919fbd8d4ea955bf175e922dcb2777fe9ef5

src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.cs
src/libraries/System.Net.Sockets/tests/FunctionalTests/SendPacketsAsync.cs

index 84ec3aa..37c9f5b 100644 (file)
@@ -460,6 +460,9 @@ namespace System.Net.Sockets
             // OK to dispose now.
             FreeInternals();
 
+            // FileStreams may be created when using SendPacketsAsync - this Disposes them.
+            FinishOperationSendPackets();
+
             // Don't bother finalizing later.
             GC.SuppressFinalize(this);
         }
@@ -599,6 +602,14 @@ namespace System.Net.Sockets
                 }
             }
 
+            switch (_completedOperation)
+            {
+                case SocketAsyncOperation.SendPackets:
+                    // We potentially own FileStreams that need to be disposed.
+                    FinishOperationSendPackets();
+                    break;
+            }
+
             Complete();
         }
 
index c51e4fd..0fd6049 100644 (file)
@@ -384,6 +384,63 @@ namespace System.Net.Sockets.Tests
             SendPackets(type, new SendPacketsElement(TestFileName, 5, 10000), SocketError.InvalidArgument, 0);
         }
 
+        [Theory]
+        [InlineData(SocketImplementationType.APM)]
+        [InlineData(SocketImplementationType.Async)]
+        [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework, "The fix was made in corefx that is not in netfx. See https://github.com/dotnet/corefx/pull/34331")]
+        public void SendPacketsElement_FileStreamIsReleasedOnError(SocketImplementationType type)
+        {
+            // this test checks that FileStreams opened by the implementation of SendPacketsAsync
+            // are properly disposed of when the SendPacketsAsync operation fails asynchronously.
+            // To trigger this codepath we must call SendPacketsAsync with a wrong offset (to create an error), 
+            // and twice (to avoid synchronous completion).
+
+            SendPacketsElement[] goodElements = new[] { new SendPacketsElement(TestFileName, 0, 0) };
+            SendPacketsElement[] badElements = new[] { new SendPacketsElement(TestFileName, 50_000, 10) };
+            EventWaitHandle completed1 = new ManualResetEvent(false);
+            EventWaitHandle completed2 = new ManualResetEvent(false);
+
+            using (SocketTestServer.SocketTestServerFactory(type, _serverAddress, out int port))
+            {
+                using (Socket sock = new Socket(AddressFamily.InterNetworkV6, SocketType.Stream, ProtocolType.Tcp))
+                {
+                    sock.Connect(new IPEndPoint(_serverAddress, port));
+                    bool r1, r2;
+                    using (SocketAsyncEventArgs args1 = new SocketAsyncEventArgs())
+                    using (SocketAsyncEventArgs args2 = new SocketAsyncEventArgs())
+                    {
+                        args1.Completed += OnCompleted;
+                        args1.UserToken = completed1;
+                        args1.SendPacketsElements = goodElements;
+
+                        args2.Completed += OnCompleted;
+                        args2.UserToken = completed2;
+                        args2.SendPacketsElements = badElements;
+
+                        r1 = sock.SendPacketsAsync(args1);
+                        r2 = sock.SendPacketsAsync(args2);
+
+                        if (r1)
+                        {
+                            Assert.True(completed1.WaitOne(TestSettings.PassingTestTimeout), "Timed out");
+                        }
+                        Assert.Equal(SocketError.Success, args1.SocketError);
+
+                        if (r2)
+                        {
+                            Assert.True(completed2.WaitOne(TestSettings.PassingTestTimeout), "Timed out");
+                        }
+                        Assert.Equal(SocketError.InvalidArgument, args2.SocketError);
+
+                        using (var fs = new FileStream(TestFileName, FileMode.Open, FileAccess.ReadWrite, FileShare.Read))
+                        {
+                            // If a SendPacketsAsync call did not dispose of its FileStreams, the FileStream ctor throws.
+                        }
+                    }
+                }
+            }
+        }
+
         #endregion Files
 
         #region Helpers