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

[Bengt Rutisson]

Thanks, Mikael, John, Jon and Thomas for the reviews!

Bengt

On 4/26/13 9:48 AM, Mikael Gerdin wrote:
> Bengt,
>
> On 2013-04-25 17:14, Bengt Rutisson wrote:
>>
>> Hi again,
>>
>> It turns out that the code that I was fixing is actually not in use.
>> Thanks Coleen for triggering me to figure that out!
>>
>> Here is an updated webrev where the code is removed instead:
>> http://cr.openjdk.java.net/~brutisso/8012915/webrev.01/
>
> This version looks even better.
>
> /m
>
>>
>> Thanks,
>> Bengt
>>
>> On 4/25/13 1:09 PM, 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
>>