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