RFR(xs) 8223336: Assert in VirtualMemoryTracker::remove_released_region when running the SharedArchiveConsistency.java test with -XX:NativeMemoryTracking=detail

[Zhengyu Gu]

Sorry. I think MemTracker::record_virtual_memory_reserve() should be 
called before ReadFile() call, so it has a record for succeeded case.

E.g.
     if (base == NULL) {
       log_info(os)("VirtualAlloc() failed: GetLastError->%ld.", 
GetLastError());
       CloseHandle(hFile);
       return NULL;
     }

     // Record virtual memory allocation
     MemTracker::record_virtual_memory_reserve((address)addr, bytes, 
CALLER_PC);

     DWORD bytes_read;
     OVERLAPPED overlapped;
     overlapped.Offset = (DWORD)file_offset;
     overlapped.OffsetHigh = 0;
     overlapped.hEvent = NULL;
     // ReadFile guarantees that if the return value is true, the requested
     // number of bytes were read before returning.
     bool res = ReadFile(hFile, base, (DWORD)bytes, &bytes_read, 
&overlapped) != 0;
     if (!res) {
       log_info(os)("ReadFile() failed: GetLastError->%ld.", 
GetLastError());
       release_memory(base, bytes);
       CloseHandle(hFile);
       return NULL;
     }


-Zhengyu


On 5/30/19 7:52 PM, Calvin Cheung wrote:
> Hi Zhengyu,
> 
> Thanks for your review.
> 
> Here's an updated webrev with your suggested change:
>      http://cr.openjdk.java.net/~ccheung/8223336/webrev.02/
> 
> thanks,
> Calvin
> 
> On 5/30/19, 4:25 PM, Zhengyu Gu wrote:
>> Hi Calvin,
>>
>> Thanks for fixing it.
>>
>> Since VirtualAlloc call has MEM_COMMIT flag set, I think it should be 
>> tracked as committed memory, e.g.
>>
>> MemTracker::record_virtual_memory_reserve_and_commit((address)addr, 
>> bytes, CALLER_PC);
>>
>> -Zhengyu
>>
>> On 5/30/19 4:04 PM, Calvin Cheung wrote:
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8223336
>>>
>>> webrev: http://cr.openjdk.java.net/~ccheung/8223336/webrev.00/
>>>
>>> A simple fix is adding the missing call to 
>>> MemTracker::record_virtual_memory_reserve() in the Windows version of 
>>> os::pd_map_memory().
>>> On platforms other than Windows, the os::pd_map_memory() doesn't call 
>>> release_memory() and the caller (os::map_memory()) calls 
>>> MemTracker::record_virtual_memory_reserve() if the returned value 
>>> from os::pd_map_memory() is non-NULL. That's why this bug reproduces 
>>> only on Windows.
>>>
>>> Testing: ran SharedArchiveConsistency.java with 
>>> -XX:NativeMemoryTracking=detail on Windows;
>>>               ran all runtime/NMT tests on Windows.
>>>
>>> thanks,
>>> Calvin