RFR (M): 8012915: ReservedSpace::align_reserved_region() broken on Windows

[Bengt Rutisson]

On 4/25/13 2:34 PM, Coleen Phillimore wrote:
> On 4/25/2013 8:15 AM, Bengt Rutisson wrote:
>>
>> Hi Coleen,
>>
>> Thanks for looking at this!
>>
>> On 4/25/13 1:54 PM, Coleen Phillimore wrote:
>>>
>>> Hi Bengt,
>>> My memory was that all that weird code for aligning windows mmap 
>>> space was for CDS.   Can you try your test with -Xshare:dump and 
>>> -Xshare:on -version?
>>> thanks,
>>
>> Seems to work:
>>
>> >hotspot.exe -Xshare:dump
>> Using java runtime at: d:\java\jdk-7-fcs-bin-b148\jre
>> Loading classes to share ... done.
>> Rewriting and linking classes ... done.
>> Number of classes 2245
>> Calculating fingerprints ... done.
>> Removing unshareable information ... done.
>> ro space:   7132288 [ 36.9% of total] out of  16777216 bytes [42.5% 
>> used] at 0x0000000800000000
>> rw space:  10270832 [ 53.2% of total] out of  16777216 bytes [61.2% 
>> used] at 0x0000000801000000
>> md space:   1881528 [  9.7% of total] out of   4194304 bytes [44.9% 
>> used] at 0x0000000802000000
>> mc space:     34053 [  0.2% of total] out of    122880 bytes [27.7% 
>> used] at 0x0000000802400000
>> total   :  19318701 [100.0% of total] out of  37871616 bytes [51.0% used]
>>
>> >hotspot.exe -Xshare:on -version
>> Using java runtime at: d:\java\jdk-7-fcs-bin-b148\jre
>> java version "1.7.0"
>> Java(TM) SE Runtime Environment (build 1.7.0-b148)
>> Java HotSpot(TM) 64-Bit Client VM (build 25.0-b30-internal-debug, 
>> mixed mode, sharing)
>>
>>
>> I have also run JPRT on my change and I think that does some CDS 
>> testing, right?
>
> Yes, JPRT does test this with a client VM.  Does your test run with 
> -Xshare:on (and hit the code that you changed)?
> thanks,

Thanks for asking this, Coleen!

It turns out that we don't hit the code. And the reason that we don't 
hit it is that it is dead code. Thanks for making me try this!

I'll send out a new webrev where this code is removed instead.

Here's why I thought this was used if anyone wonders:

This code is in use in JDK7 where PermGen still exists. I found the 
issue there and started fixing it. It turns out to be more difficult to 
fix it there, and I realized it would be a very simple fix in JDK8. I 
already had the test and ported the test to JDK8. The test reproduced 
the issue and I fixed the problem not realizing that my test was the 
only user of this code now. And I figured I'd send the change out for 
review and then go back to fixing this in JDK7.

I guess that I will now send out a review for removing this code in JDK8 
and then go back to how we should handle this in JDK7.

Thanks,
Bengt

> Coleen
>>
>> Thanks,
>> Bengt
>>
>>> Coleen
>>>
>>> On 4/25/2013 7:09 AM, Bengt Rutisson wrote:
>>>>
>>>> Hi everyone,
>>>>
>>>> Could I have a couple of reviews of this change?
>>>> http://cr.openjdk.java.net/~brutisso/8012915/webrev.00/
>>>>
>>>> Background from the CR:
>>>>
>>>> The method ReservedSpace::align_reserved_region() does not work on 
>>>> Windows. It tries to free parts of the previously allocated memory 
>>>> by doing two calls to os::release_memory(). However, on Windows 
>>>> os::release_memory() is implemented as:
>>>>
>>>>  VirtualFree(addr, 0, MEM_RELEASE)
>>>>
>>>> which will always free up all the allocated memory:
>>>>
>>>> http://msdn.microsoft.com/en-gb/library/windows/desktop/aa366892%28v=vs.85%29.aspx 
>>>>
>>>>
>>>> "The function frees the entire region that is reserved in the 
>>>> initial allocation call to VirtualAlloc."
>>>>
>>>> This means that if ReservedSpace::align_reserved_region() is 
>>>> executed on Windows and we try to trim the beginning of the memory 
>>>> that we had allocated we will free all of the allocated memory. 
>>>> Subsequent calls to os::commit_memory() which will end up as:
>>>>
>>>> VirtualAlloc(addr, bytes, MEM_COMMIT, PAGE_READWRITE)
>>>>
>>>> will fail with 487, ERROR_INVALID_ADDRESS - Attempt to access 
>>>> invalid address.
>>>>
>>>> The solution is to use the existing method 
>>>> os::reserve_memory_aligned() which does the same thing but works on 
>>>> all platforms.
>>>>
>>>> I assume that it is unusual that we enter this code path since I 
>>>> haven't seen a lot of crashes that seem related to this issue. But 
>>>> when we tried a patch that plays a bit with the alignments we crash 
>>>> on Windows. To have a simple reproducer I added a unit test. The 
>>>> test is not 100% fool proof, but on Windows without my proposed fix 
>>>> it fails 999 times out of 1000 runs. With my fix it passes 100% of 
>>>> the time.
>>>>
>>>> Thanks,
>>>> Bengt
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20130425/060d36b8/attachment.htm>