taoyouh/FtpServer

Make FtpServer disposable

govorovvs opened this issue · 3 comments

Make FtpServer disposable

The resources (TcpListener) held by FtpServer should be released when the CancellationToken in FtpServer.RunAsync() has a cancel request. There will be nothing to dispose.

@taoyouh how do you think, which code is more clean and obvious?

[Fact]
 public async Task Test()
        {
            // arrange
            var endPoint = new IPEndPoint(IPAddress.Loopback, 21);
            var ftpServer = new FtpServer(endPoint, Directory.GetCurrentDirectory());
            var cancellationTokenSource = new CancellationTokenSource();
            await ftpServer.RunAsync(cancellationTokenSource.Token);
            var ftpServerCaller = new FtpServerCaller();
            // act
            try
            {
                await ftpServerCaller.CallFtpServer();
            }
            finally
            {
                cancellationTokenSource.Cancel();
            }
            // assert
            ftpServerCaller.CheckResult();
        }

or

 [Fact]
        public async Task Test()
        {
            // arrange
            var endPoint = new IPEndPoint(IPAddress.Loopback, 21);
            using var ftpServer = new FtpServer(endPoint, Directory.GetCurrentDirectory());
            await ftpServer.RunAsync();
            var ftpServerCaller = new FtpServerCaller();
            // act
            await ftpServerCaller.CallFtpServer();
            // assert
            ftpServerCaller.CheckResult();
        }

Both version of your code will not work. You have to be running RunAsync to accept a connection. You cannot just await it (it will not complete unless you cancel it).

Maybe you prefer a .Start() and .StopAsync() pattern rather than the current Run() pattern, but that will need a new thread be created to run the main server loop. A simple wrapper could be written on the client side to implement this.

If we must use IDisposable pattern, we need to find a way to stop all existing connections synchronously (because Dispose is not async).