RFR 8067105: Socket returned by ServerSocket.accept() is inherited by child process on Windows

[FELIX YANG]

Hi Chris,
      I think it is necessary to add a regression test case for this 
issue?  There was a similar issue on DatagramSocket, 
https://bugs.openjdk.java.net/browse/JDK-4945514. 
"test/java/net/DatagramSocket/InheritHandle.java" was created for it.

Thanks,
-Felix
On 12/18/2014 10:01 PM, Chris Hegarty wrote:
> On 18 Dec 2014, at 13:20, Alan Bateman <Alan.Bateman at oracle.com> wrote:
>
>> On 18/12/2014 11:52, Chris Hegarty wrote:
>>> Thanks for looking at this Alan.
>>>
>>> Surprisingly this issue has existed for a long time. I tested with the FCS version of JRE 5, 6, 7, 8, and 9-ea, on Windows Server 2008 R2, and 2012, and the newly accepted socket inherit behaviour can be seen.  NIO server socket channels also suffers from the same problem.
>>>
>>> Since networking and NIO already sets HANDLE_FLAG_INHERIT on newly created sockets, I’d like to keep it consistent, and set it on accepted sockets too. I also think that this fix should be backpored to previous releases, so a small localised change seems appropriate.
>>>
>>> Complete and updated webrev:
>>>     http://cr.openjdk.java.net/~chegar/8067105/webrev.00/webrev/
>>>
>>> I will file a separate bug to investigate why CreateProcess is being called with bInheritHandles set to TRUE, as I think any changes here will have a wider impact.
>>>
>> I'm very surprised by this because the issue of inheriting handles into child processes has come up several times in the past. The patch you propose is okay
>>   but it doesn't deal with the case of a process being started at around the time that a connection is accepted.
> Right, and this is no different from the construction of new Sockets or SocketChannels.
>
>> The only way to deal with this type of race is to never inherit (Dmitry's point about the standard in/out/err should already be dealt with it). So yes, I think it is important to dig into some of the history and to understand whether we regressed somewhere and why we have the flag set this way.
> I have taken a look into the source code archives, and tested with releases back to 1.2. From what I can see this looks like it has been the case forever.
>
> For what it’s worth, I also remember the issue of inheriting socket handles coming up a number of times in the past, with various flavours of socket implementations, but they always seemed to centre around the creation of the socket. I think accept was just overlooked.
>
> There seems to be enough concern about root cause of this issue pointing back to the process implementation, so I will not proceed with this change, and file a separate issue against java.lang.Process. We can revisit this issue when the Process investigation is complete. Also, the networking and NIO native code should have its explicit setting of HANDLE_FLAG_INHERIT cleaned up/removed if it is deemed racy, or not needed.
>
> -Chris.