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

Daniel Fuchs daniel.fuchs at oracle.com
Fri Dec 18 11:30:03 UTC 2015


Hu Vyom,

On 17/12/15 17:58, Martin Buchholz wrote:
> Looks good!

Since you got Martin's blessing I will sponsor this
and push it for you :-)

best regards,

-- daniel

>
> On Thu, Dec 17, 2015 at 12:14 AM, vyom <vyom.tewari at oracle.com> wrote:
>> Hi Martin,
>>
>> thanks for review, i undated the
>> webrev(http://cr.openjdk.java.net/~vtewari/4823133/webrev0.2/
>> <http://cr.openjdk.java.net/%7Evtewari/4823133/webrev0.2/>) as per your
>> comment after confirming that the corresponding "fd" if opened with
>> "open64".
>>
>> Thanks,
>> Vyom
>>
>>
>>
>> On Thursday 17 December 2015 12:38 AM, Martin Buchholz wrote:
>>>
>>> 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