RFR 8191278: MappedByteBuffer bulk access memory failures are not handled gracefully

[Jamsheed]

Hi Harold,

Yes, it is known issue, i removed all @requires just for testing 
purposes. will add  "* @requires os.family != "windows" " just before 
checkin.

Best regards,

Jamsheed

On 06/06/19 12:25 AM, Harold Seigel wrote:
> Hi Jamsheed,
>
> Test hotspot/jtreg/runtime/Unsafe/InternalErrorTest.java fails on 
> Windows with:
>
>    java.nio.file.FileSystemException: The requested operation cannot 
> be performed on a file with a user-mapped section open.
>
>        at 
> java.base/sun.nio.fs.WindowsException.translateToIOException(WindowsException.java:92)
>        at 
> java.base/sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:103)
>        at 
> java.base/sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:108)
>        at 
> java.base/sun.nio.fs.WindowsFileSystemProvider.newByteChannel(WindowsFileSystemProvider.java:235)
>        at 
> java.base/java.nio.file.spi.FileSystemProvider.newOutputStream(FileSystemProvider.java:478)
>        at java.base/java.nio.file.Files.newOutputStream(Files.java:223)
>        at java.base/java.nio.file.Files.write(Files.java:3488)
>        at InternalErrorTest.main(InternalErrorTest.java:91)
>        at 
> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native 
> Method)
>        at 
> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>        at 
> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>        at java.base/java.lang.reflect.Method.invoke(Method.java:567)
>        at 
> com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127)
>        at java.base/java.lang.Thread.run(Thread.java:830)
>
> Can you add this to the test?
>
>      * @requires os.family != "windows"
>
> Thanks, Harold
>
> On 6/3/2019 2:43 PM, Jamsheed wrote:
>> Hi Dan,
>>
>> Thanks for the review.
>>
>> Best regards,
>>
>> Jamsheed
>>
>> On 01/06/19 6:29 PM, Daniel D. Daugherty wrote:
>>> That's even better. Thanks.
>>>
>>> Dan
>>>
>>>
>>> On 6/1/19 5:11 AM, Jamsheed wrote:
>>>>
>>>> Hi Dan,
>>>>
>>>> updated webrev in place
>>>>
>>>> added only assert
>>>>
>>>> +  BasicType doing_unsafe_access_bt = T_BYTE;
>>>> +  assert((sizeof(bool) * CHAR_BIT) == 8, "not implemented");
>>>>
>>>> Best regards,
>>>>
>>>> Jamsheed
>>>>
>>>> On 01/06/19 12:56 PM, Jamsheed wrote:
>>>>>
>>>>> Hi Dan,
>>>>>
>>>>> i understand it is incorrect to assume bool(cpp compiler) 
>>>>> implementation above byte level.
>>>>>
>>>>> in that case i will just put an assert.
>>>>>
>>>>> +  BasicType doing_unsafe_access_bt = T_BYTE;
>>>>>
>>>>> + assert(sizeof(bool) == 1, "not implemented");
>>>>>
>>>>> Best regards,
>>>>>
>>>>> Jamsheed
>>>>>
>>>>> On 31/05/19 10:34 PM, Jamsheed wrote:
>>>>>>> src/hotspot/share/opto/library_call.cpp
>>>>>>>     In src/hotspot/share/runtime/thread.hpp:
>>>>>>>     L1090:   volatile bool _doing_unsafe_access;
>>>>>>>
>>>>>>>     L4220:   store_to_memory(control(), 
>>>>>>> doing_unsafe_access_addr, intcon(1), T_BYTE, 
>>>>>>> Compile::AliasIdxRaw, MemNode::unordered);
>>>>>>>     L4230:   store_to_memory(control(), 
>>>>>>> doing_unsafe_access_addr, intcon(0), T_BYTE, 
>>>>>>> Compile::AliasIdxRaw, MemNode::unordered);
>>>>>>>         Is T_BYTE a safe assumption (size wise) for writing to a 
>>>>>>> C++ bool?
>>>>>>
>>>>>> Thanks for pointing this.
>>>>>>
>>>>>> hope [1] will work ?
>>>>> [1]
>>>>> +  BasicType doing_unsafe_access_bt = T_BYTE;
>>>>> +  switch(sizeof(bool)) {
>>>>> +    case 1: doing_unsafe_access_bt = T_BYTE;  break;
>>>>> +    case 2: doing_unsafe_access_bt = T_SHORT; break;
>>>>> +    case 4: doing_unsafe_access_bt = T_INT;   break;
>>>>> +    case 8: doing_unsafe_access_bt = T_LONG;  break;
>>>>> +    default: ShouldNotReachHere();
>>>>> +  }
>>>>>
>>>>>    // update volatile field
>>>>> -  store_to_memory(control(), doing_unsafe_access_addr, intcon(1), 
>>>>> T_BYTE, Compile::AliasIdxRaw, MemNode::unordered);
>>>>> +  store_to_memory(control(), doing_unsafe_access_addr, intcon(1), 
>>>>> doing_unsafe_access_bt, Compile::AliasIdxRaw, MemNode::unordered);
>>>>>
>>>>>    // Call it.  Note that the length argument is not scaled.
>>>>>    make_runtime_call(RC_LEAF|RC_NO_FP,
>>>>>                      OptoRuntime::fast_arraycopy_Type(),
>>>>>                      StubRoutines::unsafe_arraycopy(),
>>>>>                      "unsafe_arraycopy",
>>>>>                      TypeRawPtr::BOTTOM,
>>>>>                      src, dst, size XTOP);
>>>>>
>>>>> -  store_to_memory(control(), doing_unsafe_access_addr, intcon(0), 
>>>>> T_BYTE, Compile::AliasIdxRaw, MemNode::unordered);
>>>>> +  store_to_memory(control(), doing_unsafe_access_addr, intcon(0), 
>>>>> doing_unsafe_access_bt, Compile::AliasIdxRaw, MemNode::unordered); 
>>>