RFR(XS) 8234539 ArchiveRelocationTest.java failed: Archive mapping should always succeed

[Ioi Lam]

Hi Calvin,

Thanks for the review.

On 11/25/19 3:22 PM, Calvin Cheung wrote:
> Hi Ioi,
>
> This seems good.
>
> Just wondering are the following 'if' checks necessary in 
> metaspaceShared.cpp?
>
> 2155       if (static_result == MAP_ARCHIVE_SUCCESS) {
> 2156         static_result = MAP_ARCHIVE_MMAP_FAILURE;
> 2157       }
> 2158       if (dynamic_result == MAP_ARCHIVE_SUCCESS) {
> 2159         dynamic_result = MAP_ARCHIVE_MMAP_FAILURE;
> 2160       }
>

The checks for (static_result == MAP_ARCHIVE_SUCCESS) is to make sure we 
aren't in the MAP_ARCHIVE_OTHER_FAILURE state, which could happen if 
archive CRC check failed, classpath validation failed, etc.


> The checks weren't there in filemap.cpp. 

The the old code (removed by this patch) was at a point that no error 
has appended, so we are implicitly in the MAP_ARCHIVE_SUCCESS state.

> Also, the caller won't try map_archives() again if the result is not 
> MAP_ARCHIVE_MMAP_FAILURE.

That's the intended behavior. If CRC check has failed, for example, even 
if we retry mapping, we will get the same failure again.

Thanks
- Ioi

>
> thanks,
>
> Calvin
>
> On 11/22/19 5:46 PM, Ioi Lam wrote:
>> Hi Calvin,
>>
>> Thanks for the review. It turned out that I needed to fix another 
>> (addr_delta == 0) bug in the code. I've also moved the handling of 
>> ArchiveRelocationMode==1 in debug builds to 
>> MetaspaceShared::map_archives(). This way, we can simulate the 
>> "mapping failure" after all archives have been mapped. This way, we 
>> can better test the code that unmap the archives after the initial 
>> mapping failures.
>>
>> Here's the updated patch.
>> http://cr.openjdk.java.net/~iklam/jdk14/8234539-mapping-should-always-succeed.v02/ 
>>
>>
>> I am running tier4-rt-cds-relocation multiple times to make sure 
>> 8234539 is no longer triggered on Windows.
>>
>> Thanks
>> - Ioi
>>
>> On 11/22/2019 11:23 AM, Calvin Cheung wrote:
>>> Hi Ioi,
>>>
>>> The fix looks good.
>>>
>>> thanks,
>>>
>>> Calvin
>>>
>>> On 11/21/19 2:58 PM, Ioi Lam wrote:
>>>> https://bugs.openjdk.java.net/browse/JDK-8234539
>>>> http://cr.openjdk.java.net/~iklam/jdk14/8234539-mapping-should-always-succeed.v01/ 
>>>>
>>>>
>>>> This bug happens only on Windows. The fix is one-line -- in order 
>>>> to check
>>>> whether "This is the second time we try to map the archive(s)", 
>>>> instead of
>>>> using (addr_delta != 0), the correct condition is 
>>>> (rs.is_reserved()). Please
>>>> see the bug report for details.
>>>>
>>>> I also improve the log messages when error happens.
>>>>
>>>> Thanks
>>>> - Ioi
>>