On 18 Dec 2014, at 13:20, Alan Bateman <alan.bate...@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.
