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

[Leo Korinth]

On 12/03/18 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/

New webrev (only change is removing module-info.java change):
http://cr.openjdk.java.net/~lkorinth/8176717/01/

Thanks,
Leo

> 
> Testing:
> hs-tier1, hs-tier2 and TestInheritFD.java
> (on 64-bit linux, solaris, windows and mac)
> 
> Thanks,
> Leo