Fix of socket inheriting (Win)

[ihor-drachuk]

This commit closes an issue #259:
If application starts another app and then exits (leaving second app
running), listening socket created in first app is still alive (even
after app exit), but this socket isn't used by anyone and even
prevents to start listening again on the same port.

[qchateau]

Hi, thanks for your contribution,

this is part of asio's codebase, I'd not modify it except to update the asio version, or to cherry-pick a commit from their codebase.

[ihor-drachuk]

Hello!

Okay, I understand. Probably it would be better to create pull request to asio.

Thank you for reply.

[qchateau]

Maybe they already fixed it. In which case we can apply a similar patch you find the commit in asio that patched it ;)

It's nice to fix that bug, I just don't want to apply patches to asio that have not been reviewed and accepted by the asio maintainers

[ihor-drachuk]

It is reasonable, agree. I'll try to investigate is there already this fix or otherwise will create PR there.

https://github.com/boostorg/asio - Is it the source?

[thewhitegoatcb]

Edit, just realized it might be a completely unrelated issue, I have opened a pull request for the issue below.

I think it's an issue with using tcp::acceptor::reuse_address(true) on windows, bind would not fail even if the port is taken by a running server yet not accept any connections until the first server closes.
checking around it seems like there's little to no side effect omitting it on windows: warmcat/libwebsockets#65
more info on MSDN about using SO_REUSEADDR :
https://docs.microsoft.com/en-us/windows/win32/winsock/using-so-reuseaddr-and-so-exclusiveaddruse

[qchateau]

@thewhitegoatcb, @ihor-drachuk can you confirm this issue is gone now that I merged the patch on SO_REUSEADDR ?

[ihor-drachuk]

@qchateau, Hi! Thanks for attention to this problem.
Unfortunately, the issue is still present. Tested on 17ae701 (master) by the following code:

#include <QCoreApplication>
#include <QDebug>
#include <QThread>
#include <QProcess>

#include <rpc/server.h>

int main(int argc, char *argv[])
{
    QCoreApplication a(argc, argv);

    qDebug() << "App started, wait 2s";
    QThread::currentThread()->msleep(2000);

    qDebug() << "Bind to port";
    rpc::server server(2345);

    qDebug() << "Server created, wait 1s";
    QThread::currentThread()->msleep(1000);

    qDebug() << "Starting new process";
    QProcess::startDetached("this_application.exe", {});

    qDebug() << "Exitting";
    return 0;
}

Still if apply that workaround -- the problem goes away.

[qchateau]

Hum I don't expect you core to work. You start the new process when the 1st is still running so it's normal that it fails to bind, it's a sane behavior. How do you expect the 2nd process to tell that the 1st will exit soon ?

[ihor-drachuk]

1. My workaround resolves an issue. After disabling socket inheritance -- the same example works fine. Otherwise it would not work in any case.

2. Here is detailed step-by-step explanation of provided code example:

1st instance	2nd instance
Started
Wait 2 sec
Create server on 2345
Wait 1 sec
Start 2nd instance (no wait for termination)	Started
Destroy server and exit immediately	Wait 2 sec
	Here we have 2 sec of time when no one uses port 2345
	Create server on 2345 (and fail)

Hence, it should work fine, because there is approx. 2 seconds of time between destroying rpc::server and creating new one. But it doesn't work, because socket is implicitly inherited by 2nd app instance when it started, and therefore socket wasn't closed completely when 1st instance of rpc::server destroyed.

So, we have "inherited" socket, which could not be used by rpclib, could not be closed and it's not possible to create server again on same port. Actually I would say socket "leaks", because there is no any chance that 2nd app instance could use it (particularly via rpclib), but this resource (socket, port) remains busy.

Thus, if resource is inherited, but can't be used, accessed or released in any way, then it should not be inherited.

I just tried to test and report if merged patch resolved an issue and... looks like no, unfortunately. Test-code is also tested by workaround, so, looks like you can trust this investigation. The only thing I'm not sure about is... is my workaround good solution in common or not (maybe not all cases/OSes are handled well)

[winsoft666]

I run this test case on windows 11, the behavior is not expect as @ihor-drachuk say, the child process can create server successful.

App started, wait 2s
Bind to port
Server created, wait 1s
Starting new process
Exiting
App started, wait 2s
Bind to port
Server created, wait 1s
Starting new process
Exiting
App started, wait 2s
Bind to port
Server created, wait 1s
Starting new process
Exiting
App started, wait 2s
Bind to port
Server created, wait 1s
Starting new process
Exiting
App started, wait 2s
Bind to port
Server created, wait 1s
Starting new process
Exiting

......

[winsoft666]

Indeed, as @ihor-drachuk said, QProcess::startDetached will inherit parent handles to children:

//  qprocess_win.cpp Line 954
    QProcess::CreateProcessArguments cpargs = {
        nullptr, reinterpret_cast<wchar_t *>(const_cast<ushort *>(args.utf16())),
        nullptr, nullptr, true, dwCreationFlags, envPtr,
        workingDirectory.isEmpty()
            ? nullptr : reinterpret_cast<const wchar_t *>(workingDirectory.utf16()),
        &startupInfo, &pinfo
    };
    success = callCreateProcess(&cpargs);

However @ihor-drachuk's testcase still run ok on my windows 11.