8176717: GC log file handle leaked to child processes

[Langer, Christoph]

Hi Leo,

in general I think the idea is good. Speaking for AIX, the e flag is not supported but the fcntl FD_CLOEXEC should do it.

I'm wondering why you modify "src/jdk.attach/share/classes/module-info.java" and add a dependency to jdk.management? I think this should probably go into your test (if required)...

Best regards
Christoph

> -----Original Message-----
> From: hotspot-dev [mailto:hotspot-dev-bounces at openjdk.java.net] On
> Behalf Of Leo Korinth
> Sent: Montag, 12. März 2018 14:20
> To: hotspot-dev at openjdk.java.net
> Subject: RFR: 8176717: GC log file handle leaked to child processes
> 
> 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