Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop server does not free the accept socket port (on Windows) #70

Open
monsieurgustav opened this issue Aug 30, 2024 · 10 comments
Open

Comments

@monsieurgustav
Copy link

Hi,
I have an unsual use-case where I must stop and restart the FTP server during the app lifetime.
But when I stop it the 1st time, I can observe that the listening port is still busy. (using "netstat -abn -p tcp") The next start works without an error, but no connection is actually accepted.
It happens even if no client connect to the server.
Do you have a clue?
Best regards,
Guillaume

@FlorianReimold
Copy link
Member

Thanks for the report! I will take a look and try to reproduce it. I just don't have the time at the moment, so it will take be a little while. Thanks for the patience!

@FlorianReimold
Copy link
Member

Hi @monsieurgustav, when you write you "stop" the sever, what exactly are you doing? Killing an application? Deleting the C++ object?
Is it possible for you to post some code that triggers the issue?

@monsieurgustav
Copy link
Author

monsieurgustav commented Oct 17, 2024

Sure, here it is:

void restartFtpServer(std::unique_ptr<fineftp::FtpServer>& ftpServer, const QString& folder, uint16_t port, size_t threadCount = 1)
    {
        if (ftpServer)
        {
            ftpServer->stop();
            ftpServer.reset();
        }

        ftpServer = std::make_unique<fineftp::FtpServer>(port);
        ftpServer->addUserAnonymous(folder.toUtf8().toStdString(), fineftp::Permission::ReadOnly);
        ftpServer->start(threadCount);
    }

I call this function when the root folder of the FTP server changes. (I don't think there is another way to do it)

If I remember correctly, one may need to connect (anonymously) to the server before it is destroyed/restarted to trigger the issue. I use WinSCP to connect.

@FlorianReimold
Copy link
Member

I confirmed the issue with wireshark. Indeed, the socket is not closed properly. I think the reason is the following code:

void FtpServerImpl::stop()
{
io_service_.stop();
for (std::thread& thread : thread_pool_)
{
thread.join();
}
thread_pool_.clear();
}

The server stops the io_service, which makes the application to rely on the OS to clean up the sockets. And that may not happen fast enough.

I will check how to fix that bug. For future references: You can also check the existence of that bug in Wireshark, as it will show the improperly terminated connection in red:
image

My test application looks like follows (Windows only, due to hacky system commands):

#include <chrono>
#include <fineftp/server.h>

#include <string>
#include <thread>

void restartFtpServer(std::unique_ptr<fineftp::FtpServer>& ftpServer, const std::string& folder, uint16_t port, size_t threadCount = 1)
{
  if (ftpServer)
  {
    ftpServer->stop();
    ftpServer.reset();
  }

  ftpServer = std::make_unique<fineftp::FtpServer>(port);
  ftpServer->addUserAnonymous(folder, fineftp::Permission::ReadOnly);
  ftpServer->start(threadCount);
}


int main() {
  std::unique_ptr<fineftp::FtpServer> ftpServer;

  restartFtpServer(ftpServer, "C:\\", 2121, 1);

  system("pause");

  restartFtpServer(ftpServer, "D:\\", 2121, 1);

  system("pause");
}

@monsieurgustav
Copy link
Author

Did you try to pause between the stop/start and wait a long time? (30s? 1mn?)
Does the socket/port frees itself at some point?

@FlorianReimold
Copy link
Member

FlorianReimold commented Oct 23, 2024

Actually, I did not experience any issue. In my test, fineftp was able to re-open the port immediately (code above). I didn't check netstat though, as I already confirmed the bug with Wireshark.

What needs to be done is entirely drop the io_service_.stop();. Without the io_service, sockets will not be able to properly shut down. Stopping the FTP Server must do the following instead:

  1. Prevent new sessions from being accepted
  2. Stop all existing FTP sessions
  3. Join all threads without stopping the io_service. This will wait for the io_service to run out of work on its own.

@FlorianReimold
Copy link
Member

FlorianReimold commented Nov 6, 2024

@monsieurgustav: I fixed the socket-close issue in a branch: https://github.com/eclipse-ecal/fineftp-server/tree/hotfix/properly_close_sockets

It's not perfect, yet, but ready for a test. Can you test it out?

Btw, I am not sure if it actually fixes your issue, but it is worth a try. I found that you can actually keep the socket open from the outside (e.g. using curl) and I am unsure whether that is something caused by the Windows Kernel or by the fineftp server library. So maybe while trying to fix your issue I fixed an unrelated bug. Let's see.

Either way, here is a Powershell command that shows all open ports for an app (here fineftp_example):

Get-NetTCPConnection | Where-Object { (Get-Process | Where-Object { $_.Name -eq 'fineftp_example' }).Id -eq $_.OwningProcess }

@monsieurgustav
Copy link
Author

Hi Florian,
I'm afraid it does not fix my issue.
It also crashes in release. When running release with debug symbols, I get this logs:

onecoreuap\internal\shell\inc\SrcPkg\FileExplorerSessionWatcher\inc\FileExplorerSessionWatcher.h(696)\SHELL32.dll!00007FFB133FE944: (caller: 00007FFB13343122) ReturnHr(1) tid(b988) 80004001 Non implémenté
shell\SrcPkg\FileExplorer\DefView\src\DefView.cpp(17508)\SHELL32.dll!00007FFB13343141: (caller: 00007FFADC25429E) LogHr(1) tid(b988) 80004001 Non implémenté
onecoreuap\internal\shell\inc\SrcPkg\FileExplorerSessionWatcher\inc\FileExplorerSessionWatcher.h(696)\SHELL32.dll!00007FFB133FE944: (caller: 00007FFB13343122) ReturnHr(2) tid(b988) 80004001 Non implémenté
shell\SrcPkg\FileExplorer\DefView\src\DefView.cpp(17508)\SHELL32.dll!00007FFB13343141: (caller: 00007FFADC25429E) LogHr(2) tid(b988) 80004001 Non implémenté
HEAP[BackdropLauncher.exe]: Heap block at 000001EAA9EEBF60 modified at 000001EAA9EEBF78 past requested size of 8

Unless you see another benefit, I suggest leaving this fix out.

@monsieurgustav
Copy link
Author

Based on this discussion, I tried something else:

    void restartFtpServer(std::unique_ptr<fineftp::FtpServer>& ftpServer, const QString& folder, uint16_t port, size_t threadCount = 1)
    {
        if (ftpServer)
        {
            ftpServer->stop();
            ftpServer.reset();
        }

        QTimer::singleShot(0, [&ftpServer, folder, port, threadCount] {
            ftpServer = std::make_unique<fineftp::FtpServer>(port);
            ftpServer->addUserAnonymous(folder.toUtf8().toStdString(), fineftp::Permission::ReadOnly);
            ftpServer->start(threadCount);
        });
    }

Just in case you don't read Qt code: I wait an event loop iteration between the destruction and the re-creation of the server.
And that works apparently... The FTP client reconnects automatically to the new server, adjusting the root folder as expected.

It's not ideal, I'd like to know how it is different, but I also don't care that much and I can live with it!

So I close the issue, feel free to re-open it if you want to investigate more.

@FlorianReimold
Copy link
Member

I am still investigating. I used a lot of Qt in the past and still maintain Qt projects, so I know what you are doing there. I just translated your restartFtpServer function in plain C++ so I don't need to create a Qt project.

Maybe you can answer a few questions, as I totally fail to reproduce your issue (Wrote a test that restarts the server 1000 times and accessed it with CURL and WinSCP in parallel, still no issue):

  • Which OS are your running on?
  • Which compiler are you using in which version?
  • For which CPU architeture do you compile?
  • Are you using asio from the fineftp submodule or are you using some other version?
  • Which port do you use for the FTP Server
  • Do you bind the FTP Server to an IP?
  • How many FTP Server objects are you creating?
  • Are your clients running on localhost, or do you access the server via network?
  • How many clients are accessing the server at the time of restarting?
  • What are the clients doing when the server restarts?
  • Are the clients invoked programmatically, or are you using WinSCP from the GUI just as I do?
  • Are you sure that your crash comes from fineFTP? I don't see any log that indicates that.

I pushed another commit to the same branch. This time I didn't aim to fix the issue but actually make it worse. Before, fineFTP opened sockets shared, so multiple server could be opened on the same port (which doesn't really make sense). I removed that code, so now you should be confronted with the following error message, if fineFTP is supposed to open a port that is already in use:
Error binding acceptor: Typically, each socket address (protocol, network address, or port) may only be used once.

@FlorianReimold FlorianReimold reopened this Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants