RFR 4823133: RandomAccessFile.length() is not thread-safe
Martin Buchholz
martinrb at google.com
Wed Dec 16 19:08:44 UTC 2015
Calling naked fstat without _FILE_OFFSET_BITS=64 is a bug. Don't you
need to call fstat64 if it's available?
+jlong
+handleGetLength(FD fd)
+{
+ struct stat sb;
+ if (fstat(fd, &sb) == 0) {
+ return sb.st_size;
+ } else {
+ return -1;
+ }
+}
On Wed, Dec 16, 2015 at 5:02 AM, vyom <vyom.tewari at oracle.com> wrote:
> Hi,
>
> Updated the webrev(http://cr.openjdk.java.net/~vtewari/4823133/webrev0.1/
> <http://cr.openjdk.java.net/%7Evtewari/4823133/webrev0.1/>) as per Martin
> review comment removed the _FILE_OFFSET_BITS from source.
>
> Thanks,
> Vyom
>
>
>
> On Tuesday 15 December 2015 10:55 PM, 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.
>>
>>
>>
>>
>> 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
>>>>
>>>>
>>>>
>>>>
>
More information about the core-libs-dev
mailing list