RFR(S) 8216184: CDS/appCDS tests failed on Windows due to long path to a classlist file

[David Holmes]

On 11/01/2019 11:59 am, Ioi Lam wrote:
> 
> On 1/10/19 5:42 PM, David Holmes wrote:
>> On 11/01/2019 11:23 am, Ioi Lam wrote:
>>> Hi Calvin,
>>>
>>> Maybe you can use os::fopen() instead of 2 calls to overloaded 
>>> versions of os::open()?
>>
>> os::fopen still calls ::fopen so will have the exact same problem on 
>> Windows. os::fopen only exists to add close-on-exec behaviour.
>>
> Ah, maybe we should add a comment in the new code to explain why this 
> gymnastic is necessary?

+1

That said, from what I've read it should suffice to pre-pend "\\?\" 
provided we use absolute paths:

https://docs.microsoft.com/en-us/windows/desktop/FileIO/naming-a-file#maximum-path-length-limitation

The os::open fix for 8188122 is rather more complicated though, so 
perhaps simplest to just use that as-is.

David

> Thanks
> 
> - Ioi
> 
>> There are numerous uses of plain fopen in shared code in the VM, many 
>> of which may also be problematic.
>>
>> In relation to CDS, this comment in
>>
>> ./share/memory/metaspaceShared.cpp
>>
>> seems out-of-place and likely no longer relevant, as I see no fopern 
>> use there
>>
>>     // Preload classes to be shared.
>>     // Should use some os:: method rather than fopen() here. aB.
>>
>> Otherwise looks good to me.
>>
>> Thanks,
>> David
>> -----
>>
>>> The rest looks good.
>>>
>>> Thanks
>>>
>>> - Ioi
>>>
>>> On 1/10/19 12:08 PM, Calvin Cheung wrote:
>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8216184
>>>>
>>>> webrev: http://cr.openjdk.java.net/~ccheung/8216184/webrev.00/
>>>>
>>>> We are using fopen() to open a classlist file. fopen() can't handle 
>>>> long path name longer than MAX_PATH.
>>>>
>>>> A fix is to use os::open() which is capable of handling long path 
>>>> name after the fix for JDK-8188122.
>>>>
>>>> Testing: mach5 hs-tier{1,2,3}.
>>>>
>>>> thanks,
>>>> Calvin