Make FtpServer disposable
govorovvs opened this issue · 3 comments
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).