RFR: 8176717: GC log file handle leaked to child processes

[Robbin Ehn]

Hi Leo,

I looked at http://cr.openjdk.java.net/~lkorinth/8176717/02/.

I think it would be good with some sort of error checking if fcntl failed, maybe 
just an assert.
1282     if (fd != -1) {
1283       fcntl(fd, F_SETFD, fcntl(fd, F_GETFD) | FD_CLOEXEC);
1284     }

Otherwise looks good, thanks, Robbin.

On 2018-03-12 14:20, Leo Korinth wrote:
> Hi,
> 
> This fix is for all operating systems though the problem only seams to appear on 
> windows.
> 
> I am creating a proxy function for fopen (os::fopen_retain) that appends the 
> non-standard "e" mode for linux and bsds. For windows the "N" mode is used. For 
> other operating systems, I assume that I can use fcntl F_SETFD FD_CLOEXEC. I 
> think this will work for AIX, Solaris and other operating systems that do not 
> support the "e" flag. Feedback otherwise please!
> 
> The reason that I use the mode "e" and not only fcntl for linux and bsds is 
> threefold. First, I still need to use mode flags on windows as it does not 
> support fcntl. Second, I probably save a system call. Third, the change will be 
> applied directly, and there will be no point in time (between system calls) when 
> the process can leak the file descriptor, so it is safer.
> 
> The test case forks three VMs in a row. By doing so we know that the second VM 
> is opened with a specific log file. The third VM should have less open file 
> descriptors (as it is does not use logging) which is checked using a 
> UnixOperatingSystemMXBean. This is not possible on windows, so on windows I try 
> to rename the file, which will not work if the file is opened (the actual reason 
> the bug was opened).
> 
> The added test case shows that the bug fix closes the log file on windows. The 
> VM on other operating systems closed the log file even before the fix.
> 
> Maybe the test case should be moved to a different path?
> 
> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8176717
> https://bugs.openjdk.java.net/browse/JDK-8176809
> 
> Webrev:
> http://cr.openjdk.java.net/~lkorinth/8176717/00/
> 
> Testing:
> hs-tier1, hs-tier2 and TestInheritFD.java
> (on 64-bit linux, solaris, windows and mac)
> 
> Thanks,
> Leo