Fix windows issue with multiple workers

[Maxim Dounin]

Hello!

On Thu, Jun 18, 2015 at 11:23:37AM +0200, Sergey Brester wrote:

> So, in VM it work for me also.
> 
> I'm assuming that something on my windows work-pc has prevented to inherit
> listener in this way (driver, LSPs installed (Layered Service Providers),
> antivirus or something else)...

Quick search suggests that yes, some LSPs can break socket handle 
inheritance, see, e.g.:

http://stackoverflow.com/questions/11847793/are-tcp-socket-handles-inheritable
http://stackoverflow.com/questions/12058911/can-tcp-socket-handles-be-set-not-inheritable

We probably want to dig a bit more into this, to make sure 
common LSPs can be easily switched off to won't interfere with 
nginx.

> But, why don't you want to use a suggested solution of me?

As I already tried to explain, the approach with inherited sockets 
is basically identical to what nginx does on UNIX with fork().  
There are no reasons to keep things different if you can do it 
similarly on both platforms.

As long as the problem with LSPs isn't common and/or solvable, I 
would prefer the inherited sockets approach to the alternatives.

This approach is also required to upgrade executable on the fly 
(though supporting this on Windows will likely require more work).

> If I will realize the way with "easy" inheritance (with "bInheritHandle"
> through CreateProcess), it will be not really easier, because:
> 
> - we have several listener to share, so we should tell all this handles to
> child process;

That's already implemented, see above.

> - bInheritHandle=True in CreateProcess can be a potential risk by not closed
> handles, if process crashed, and that are not only sockets - thus will arise
> so-called zombi-handles as half-open (dropped) or half-closed. But for
> sockets are listening it is extrem. Here is an example when this situation
> is encountered (* - listener, which process does not exist):
> 
> netstat /ano | grep 0.0:80
>  * TCP    0.0.0.0:80             0.0.0.0:0              LISTENING       3824
>    TCP    0.0.0.0:80             0.0.0.0:0              LISTENING       4378
> 
> taskkill /F /PID 3824
> ERROR: The process "3824" not found.

As far as I understand, this just means that the process that 
created the socket already exited.  It doesn't mean that the 
socket isn't used at all - as long as you properly track inherited 
sockets.  And this is what nginx already knows how to do.

> Unfortunately, it is not guaranteed that new process 4378 accepts
> connections (because "zombi" listener of 3824 can block it).
> But also not so good are another zombies, like not closed temp-files,
> lock-files, pipes etc.

As far as I understand, no other handles are inheritable by 
default, sockets are the only exception.

> You can talk long about "that would be windows bugs", but that are facts.
> And thus it is instable. Apart from, does not work at all on some mashines
> (like my work-pc).
> And the way with WSADuplicateSocket self Microsoft recommends in various
> articles.

So, the only real problem seems to be LSPs.

> If you still want to use the solution with "bInheritHandle", I suggest a
> compromise:
> I will make it with selectable option (resp. defines like
> NGX_WIN32_DUPLICATE_LISTEN and NGX_WIN32_INHERIT_LISTEN).

The goal is to minimize code bloat, not maximize it.

-- 
Maxim Dounin
http://nginx.org/