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