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

[vyom]

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
>>>>>
>>>>>
>>>>>
>>>>>