Unhandled exception in FtpWebRequest.CreateConnectionAsync crashes process

[loop-evgeny]

Description

When System.Net.FtpWebRequest.CreateConnectionAsync throws an exception in creating a TcpClient the exception is unhandled, because it happens on a thread-pool thread, so this crashes the whole process. This happens when the open file handle limit is reached (and possibly in other scenarios).

Console EXE code to reproduce:

using System;
using System.Diagnostics;
using System.Net;
using System.Threading.Tasks;

namespace FtpWebRequestCrash
{
    class Program
    {
        static void Main(string[] args)
        {
            TryGetFileList(); // Do this before setting ulimit to load all assemblies, etc., otherwise that fails

            Console.WriteLine("Ready to try FTP request again. Run\n\tprlimit --nofile=10:10 --pid {0}\nand press Enter to continue.", Process.GetCurrentProcess().Id);
            Console.ReadLine();

            TryGetFileList(); // If ulimit was lowered this will now crash the process

            Console.WriteLine("Exiting normally");
        }

        private static void TryGetFileList()
        {
            try
            {
                var request = (FtpWebRequest)WebRequest.Create("ftp://www.example.com/test");
                request.Method = WebRequestMethods.Ftp.ListDirectoryDetails;

                var task = request.GetResponseAsync();
                if (!Task.WaitAll(new Task[] {task}, 1000)) // Work around .NET Core ignoring Timeout property: https://github.com/dotnet/corefx/issues/35888
                    throw new TimeoutException($"FTP request to {request.RequestUri} timed out after 1000 ms");

                Console.WriteLine("Connected successfully - this should not happen");
            }
            catch (Exception ex)
            {
                Console.WriteLine("Exception in getting file list: " + ex);
            }
        }
    }
}

Project file:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net5.0</TargetFramework>
  </PropertyGroup>

</Project>

Run the above with dotnet run and it should output:

Exception in getting file list: System.TimeoutException: FTP request to ftp://www.example.com/test timed out after 1000 ms
   at FtpWebRequestCrash.Program.TryGetFileList() in /media/sf_work/Play/FtpWebRequestCrash/Program.cs:line 31
 Ready to try FTP request again. Run
	prlimit --nofile=10:10 --pid 58520
 and press Enter to continue.

Run the command above (prlimit --nofile=10:10 --pid 58520 or alternatively prlimit --nofile=10:10 --pid $(pgrep -f FtpWebRequestCrash)) in another terminal, then press Enter in the running program and it crashes with an unhandled exception:

Unhandled exception. System.Net.Sockets.SocketException (23): Too many open files in system
   at System.Net.Sockets.Socket..ctor(AddressFamily addressFamily, SocketType socketType, ProtocolType protocolType)
   at System.Net.Sockets.Socket..ctor(SocketType socketType, ProtocolType protocolType)
   at System.Net.Sockets.TcpClient.InitializeClientSocket()
   at System.Net.Sockets.TcpClient..ctor(AddressFamily family)
   at System.Net.Sockets.TcpClient..ctor()
   at System.Net.FtpWebRequest.CreateConnectionAsync()
   at System.Threading.Tasks.Task.<>c.<ThrowAsync>b__140_1(Object state)
   at System.Threading.QueueUserWorkItemCallbackDefaultContext.Execute()
   at System.Threading.ThreadPoolWorkQueue.Dispatch()
   at System.Threading._ThreadPoolWaitCallback.PerformWaitCallback()

Expected result: exception is handled, i.e.

Exception in getting file list: System.TimeoutException: FTP request to ftp://www.example.com/test timed out after 1000 ms
   at FtpWebRequestCrash.Program.TryGetFileList() in /media/sf_work/Play/FtpWebRequestCrash/Program.cs:line 31
Ready to try FTP request again. Run
	prlimit --nofile=10:10 --pid 58615
and press Enter to continue.

Exception in getting file list: System.Net.Sockets.SocketException (23): Too many open files in system
   at System.Net.Sockets.Socket..ctor(AddressFamily addressFamily, SocketType socketType, ProtocolType protocolType)
   ...
Exiting normally

Configuration

$ dotnet --info
.NET SDK (reflecting any global.json):
 Version:   5.0.302
 Commit:    c005824e35

Runtime Environment:
 OS Name:     linuxmint
 OS Version:  20
 OS Platform: Linux
 RID:         linux-x64
 Base Path:   /usr/share/dotnet/sdk/5.0.302/

Host (useful for support):
  Version: 5.0.8
  Commit:  35964c9215

.NET SDKs installed:
  5.0.302 [/usr/share/dotnet/sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 3.1.17 [/usr/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 5.0.8 [/usr/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 3.1.17 [/usr/share/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 5.0.8 [/usr/share/dotnet/shared/Microsoft.NETCore.App]

Other information

I believe the problem is that FtpWebRequest.CreateConnectionAsync() runs new TcpClient() outside of its try/catch. The entire method body should probably be inside a try/catch. Please handle AsyncRequestCallback throwing, too.

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

When System.Net.FtpWebRequest.CreateConnectionAsync throws an exception in creating a TcpClient the exception is unhandled, because it happens on a thread-pool thread, so this crashes the whole process. This happens when the open file handle limit is reached (and possibly in other scenarios).

Console EXE code to reproduce:

using System;
using System.Diagnostics;
using System.Net;
using System.Threading.Tasks;

namespace FtpWebRequestCrash
{
    class Program
    {
        static void Main(string[] args)
        {
            TryGetFileList(); // Do this before setting ulimit to load all assemblies, etc., otherwise that fails

            Console.WriteLine("Ready to try FTP request again. Run\n\tprlimit --nofile=10:10 --pid {0}\nand press Enter to continue.", Process.GetCurrentProcess().Id);
            Console.ReadLine();

            TryGetFileList(); // If ulimit was lowered this will now crash the process

            Console.WriteLine("Exiting normally");
        }

        private static void TryGetFileList()
        {
            try
            {
                var request = (FtpWebRequest)WebRequest.Create("ftp://www.example.com/test");
                request.Method = WebRequestMethods.Ftp.ListDirectoryDetails;

                var task = request.GetResponseAsync();
                if (!Task.WaitAll(new Task[] {task}, 1000)) // Work around .NET Core ignoring Timeout property: https://github.com/dotnet/corefx/issues/35888
                    throw new TimeoutException($"FTP request to {request.RequestUri} timed out after 1000 ms");

                Console.WriteLine("Connected successfully - this should not happen");
            }
            catch (Exception ex)
            {
                Console.WriteLine("Exception in getting file list: " + ex);
            }
        }
    }
}

Project file:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net5.0</TargetFramework>
  </PropertyGroup>

</Project>

Run the above with dotnet run and it should output:

Exception in getting file list: System.TimeoutException: FTP request to ftp://www.example.com/test timed out after 1000 ms
   at FtpWebRequestCrash.Program.TryGetFileList() in /media/sf_work/Play/FtpWebRequestCrash/Program.cs:line 31
 Ready to try FTP request again. Run
	prlimit --nofile=10:10 --pid 58520
 and press Enter to continue.

Run the command above (prlimit --nofile=10:10 --pid 58520 or alternatively prlimit --nofile=10:10 --pid $(pgrep -f FtpWebRequestCrash)) in another terminal, then press Enter in the running program and it crashes with an unhandled exception:

Unhandled exception. System.Net.Sockets.SocketException (23): Too many open files in system
   at System.Net.Sockets.Socket..ctor(AddressFamily addressFamily, SocketType socketType, ProtocolType protocolType)
   at System.Net.Sockets.Socket..ctor(SocketType socketType, ProtocolType protocolType)
   at System.Net.Sockets.TcpClient.InitializeClientSocket()
   at System.Net.Sockets.TcpClient..ctor(AddressFamily family)
   at System.Net.Sockets.TcpClient..ctor()
   at System.Net.FtpWebRequest.CreateConnectionAsync()
   at System.Threading.Tasks.Task.<>c.<ThrowAsync>b__140_1(Object state)
   at System.Threading.QueueUserWorkItemCallbackDefaultContext.Execute()
   at System.Threading.ThreadPoolWorkQueue.Dispatch()
   at System.Threading._ThreadPoolWaitCallback.PerformWaitCallback()

Expected result: exception is handled, i.e.

Exception in getting file list: System.TimeoutException: FTP request to ftp://www.example.com/test timed out after 1000 ms
   at FtpWebRequestCrash.Program.TryGetFileList() in /media/sf_work/Play/FtpWebRequestCrash/Program.cs:line 31
Ready to try FTP request again. Run
	prlimit --nofile=10:10 --pid 58615
and press Enter to continue.

Exception in getting file list: System.Net.Sockets.SocketException (23): Too many open files in system
   at System.Net.Sockets.Socket..ctor(AddressFamily addressFamily, SocketType socketType, ProtocolType protocolType)
   ...
Exiting normally

Configuration

$ dotnet --info
.NET SDK (reflecting any global.json):
 Version:   5.0.302
 Commit:    c005824e35

Runtime Environment:
 OS Name:     linuxmint
 OS Version:  20
 OS Platform: Linux
 RID:         linux-x64
 Base Path:   /usr/share/dotnet/sdk/5.0.302/

Host (useful for support):
  Version: 5.0.8
  Commit:  35964c9215

.NET SDKs installed:
  5.0.302 [/usr/share/dotnet/sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 3.1.17 [/usr/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 5.0.8 [/usr/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 3.1.17 [/usr/share/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 5.0.8 [/usr/share/dotnet/shared/Microsoft.NETCore.App]

Other information

I believe the problem is that FtpWebRequest.CreateConnectionAsync() runs new TcpClient() outside of its try/catch. The entire method body should probably be inside a try/catch. Please handle AsyncRequestCallback throwing, too.

Author:	loop-evgeny
Assignees:	-
Labels:	area-System.Net, untriaged
Milestone:	-

[karelz]

Triage: Easier to fix than to triage and punt :)

[loop-evgeny]

Wow, a fix in less than a day! Thanks very much, @stephentoub . Will this be in 6.0.0-preview.8 ?

[karelz]

Yes, it will be in Preview8 (aka RC1)