RFR 4823133: RandomAccessFile.length() is not thread-safe

[Magnus Ihse Bursie]

On 2015-12-15 18:25, Martin Buchholz wrote:
> _FILE_OFFSET_BITS is generally an all-or-nothing thing, because it
> affects interoperability between translation units.  It would be good
> to convert all of the JDK build to use -D_FILE_OFFSET_BITS=64, but
> that would be a big job.  So traditionally the JDK has instead used
> the functions made available via _LARGEFILE64_SOURCE.  But that is
> also a JDK-wide job now, because every call to plain stat in the
> source code is broken on 32-bit systems with 64-bit inodes, which are
> becoming more common.
>
> I recommend the _FILE_OFFSET_BITS=64 strategy, but it's probably a job
> for build-dev, not core-libs-dev.

If I understand things correctly, then setting _FILE_OFFSET_BITS=64 
would mean two things:
1) Just pass this additional define to all C files being compiled (a 
build change)
2) Update all calls to file operating functions to make sure the proper 
64-bit types are used (changes in source code).

The change for 1) is a trivial one liner, while getting 2) correct is a 
serious undertaking in the source code. Such a change must be done by 
the component teams, not the build team. With that said, I think it is a 
reasonable change and I'll support it as much as I can. But as the 
division of labour is done here, it's not the work of the build team to 
go and make heavy modifications to the product source code.

/Magnus

>
>
>
>
> On Tue, Dec 15, 2015 at 8:31 AM, Roger Riggs <Roger.Riggs at oracle.com> wrote:
>> Hi Yvom,
>>
>> Minor comments:
>>
>> src/java.base/share/native/libjava/RandomAccessFile.c:
>>   - "length fail" might be clearer as "GetLength failed"
>>
>> src/java.base/unix/native/libjava/io_util_md.c:
>>
>>   - Please add a comment before the define of FILE_OFFSET_BITS to indicate
>> where it is used and why it is there.
>>   - BTW, are there any unintended side effects?
>>     Perhaps a different issue but perhaps 64 bit offsets should be used
>> everywhere
>>
>> src/java.base/windows/native/libjava/io_util_md.c
>>   - Line 592: Using INVALID_HANDLE_VALUE is better than -1 and is used
>> elsewhere in the file
>>     BTW, Testing for invalid handle might be unnecessary since the call to
>> GetFileSizeEx will fail
>>     if it is invalid, yielding the same result.
>>
>> Roger
>>
>>
>> On 12/10/2015 5:52 AM, vyom wrote:
>>> Hi All,
>>>
>>> Please review my changes for below bug.
>>>
>>> Bug: JDK-4823133 : RandomAccessFile.length() is not thread-safe
>>>
>>> Webrev:http://cr.openjdk.java.net/~vtewari/4823133/webrev0.0/
>>> <http://cr.openjdk.java.net/%7Evtewari/4823133/webrev0.0/>
>>>
>>> This change ensure that  length() does not temporarily changes the file
>>> pointer and it will make sure that there is no race
>>> condition in case of multi thread uses.
>>>
>>> Thanks,
>>> Vyom
>>>
>>>
>>>
>>>