8176717: GC log file handle leaked to child processes

[Leo Korinth]

On 12/03/18 14:38, Langer, Christoph wrote:
> 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)...

Yes this is so very very wrong, *thank you so much* for spotting it.

Thanks,
Leo
> 
> 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