Fixes and verbose reporting about re-reserving same region

dynamic-memory-api-0

@@ -1 +1 @@
-Subproject commit 4d5f71df4bf191255cfcf1f5ae82d8b8c2183aa9
+Subproject commit c60ffac4f647d876144bbd92cbada5fb4b648afa

src/api.rs

@@ -1,17 +1,21 @@
 use crate::reservation::WindowsReservation;
 use crate::winapi_wrappers;
-use dynamic_memory_api_0::virtual_memory::{ReservationToExtend, VirtualMemoryApi};
+use dynamic_memory_api_0::virtual_memory::{ExtendResult, VirtualMemoryApi};
+use std::cmp::max;
 use std::sync::atomic::{AtomicUsize, Ordering};
 
 pub struct WindowsVirtualMemoryApi {
     pageSize: usize,
+    allocationGranularity: usize,
     totalReservationsOwned: AtomicUsize,
 }
 
 impl WindowsVirtualMemoryApi {
     pub fn startScope() -> Self {
+        let si = winapi_wrappers::getSystemInfo();
         return Self {
-            pageSize: winapi_wrappers::getPageSize(),
+            pageSize: si.pageSize,
+            allocationGranularity: max(si.allocationGranularity, si.pageSize),
             totalReservationsOwned: AtomicUsize::new(0),
         };
     }
@@ -31,6 +35,10 @@ impl WindowsVirtualMemoryApi {
             }
         }
     }
+
+    fn roundUpPages(&self, pagesCount: usize) -> usize {
+        return (pagesCount * self.pageSize + (self.allocationGranularity - 1)) / self.pageSize;
+    }
 }
 
 unsafe impl VirtualMemoryApi for WindowsVirtualMemoryApi {
@@ -40,12 +48,22 @@ unsafe impl VirtualMemoryApi for WindowsVirtualMemoryApi {
         return self.pageSize;
     }
 
-    fn reserveMemory<'s>(
+    fn reserveMemory<'s>(&'s self, pagesCount: usize) -> Option<Self::Reservation<'s>> {
+        match WindowsReservation::tryReserve(self, None, self.roundUpPages(pagesCount)) {
+            ExtendResult::OutOfMemory => return None,
+            ExtendResult::ContinuationIsBusy => {
+                panic!("Unreachable (system reported that region is busy, but it wasn't specified)")
+            }
+            ExtendResult::Success(r) => return Some(r),
+        }
+    }
+
+    unsafe fn extendReservation<'s>(
         &'s self,
-        extend: ReservationToExtend<&Self::Reservation<'s>>,
+        base: &Self::Reservation<'s>,
         pagesCount: usize,
-    ) -> Option<Self::Reservation<'s>> {
+    ) -> ExtendResult<WindowsReservation<'s>> {
-        return WindowsReservation::tryReserve(self, extend, pagesCount);
+        return WindowsReservation::tryReserve(self, Some(base), self.roundUpPages(pagesCount));
     }
 }
 

src/reservation.rs

@@ -1,5 +1,7 @@
-use crate::{winapi_wrappers, WindowsVirtualMemoryApi};
+use crate::{WindowsVirtualMemoryApi, winapi_wrappers};
-use dynamic_memory_api_0::virtual_memory::{Reservation, ReservationToExtend, VirtualMemoryApi};
+use dynamic_memory_api_0::virtual_memory::{
+    ExtendResult, Reservation, VirtualMemoryApi,
+};
 use std::cmp::Ordering;
 use std::ffi::c_void;
 use std::mem::ManuallyDrop;
@@ -16,16 +18,10 @@ pub struct WindowsReservation<'s> {
 impl<'s> WindowsReservation<'s> {
     pub(super) fn tryReserve(
         owner: &'s WindowsVirtualMemoryApi,
-        extend: ReservationToExtend<&WindowsReservation<'s>>,
+        extend: Option<&WindowsReservation<'s>>,
         pagesCount: usize,
-    ) -> Option<Self> {
+    ) -> ExtendResult<Self> {
-        let start: Option<NonNull<c_void>>;
+        let start: Option<NonNull<c_void>> = extend.map(|pg| unsafe { pg._offset(pg.pagesCount) });
-        match extend {
-            ReservationToExtend::ReserveAnywhere => start = None,
-            ReservationToExtend::TryExtend(prev) => unsafe {
-                start = Some(prev._offset(prev.pagesCount))
-            },
-        };
 
         let ptr_o;
         unsafe {
@@ -34,8 +30,11 @@ impl<'s> WindowsReservation<'s> {
 
         let ptr;
         match ptr_o {
-            None => return None,
+            ExtendResult::OutOfMemory => return ExtendResult::OutOfMemory,
-            Some(ptr_nn) => ptr = unsafe { NonNull::new_unchecked(ptr_nn.as_ptr()) },
+            ExtendResult::ContinuationIsBusy => return ExtendResult::ContinuationIsBusy,
+            ExtendResult::Success(ptr_nn) => {
+                ptr = unsafe { NonNull::new_unchecked(ptr_nn.as_ptr()) }
+            }
         }
 
         owner.changeReservationsCount(1);
@@ -46,7 +45,7 @@ impl<'s> WindowsReservation<'s> {
             pagesCount: pagesCount,
         };
 
-        return Some(reservation);
+        return ExtendResult::Success(reservation);
     }
 
     unsafe fn _offset(&self, index: usize) -> NonNull<c_void> {
@@ -72,8 +71,8 @@ impl<'s> WindowsReservation<'s> {
     }
 
     unsafe fn _release(&mut self) {
-        winapi_wrappers::release(self.start, self._size(0, self.pagesCount));
         self.owner.changeReservationsCount(-1);
+        winapi_wrappers::release(self.start);
     }
 }
 
@@ -97,18 +96,15 @@ impl PartialOrd<Self> for WindowsReservation<'_> {
     }
 }
 
-impl Drop for WindowsReservation<'_> {
-    fn drop(&mut self) {
-        unsafe {
-            self._release();
-        }
-        if !panicking() {
-            panic!("Reserved virtual memory wasn't released explicitly");
-        }
-    }
-}
-
 unsafe impl Reservation for WindowsReservation<'_> {
+    fn getPageSize(&self) -> usize {
+        return self.owner.getPageSize();
+    }
+
+    fn pagesCount(&self) -> usize {
+        return self.pagesCount;
+    }
+
     unsafe fn commitPages(&mut self, indexOfFirst: usize, count: usize) -> *mut [u8] {
         self._assertBounds(indexOfFirst, count);
         let start = self._offset(indexOfFirst);
@@ -119,10 +115,7 @@ unsafe impl Reservation for WindowsReservation<'_> {
 
     unsafe fn decommitPages(&mut self, indexOfFirst: usize, count: usize) {
         self._assertBounds(indexOfFirst, count);
-        winapi_wrappers::decommit(
+        winapi_wrappers::decommit(self._offset(indexOfFirst), self._size(indexOfFirst, count))
-            self._offset(indexOfFirst),
-            self._size(indexOfFirst, count),
-        )
     }
 
     unsafe fn release(mut self) {
@@ -130,3 +123,14 @@ unsafe impl Reservation for WindowsReservation<'_> {
         let _ = ManuallyDrop::new(self);
     }
 }
+
+impl Drop for WindowsReservation<'_> {
+    fn drop(&mut self) {
+        unsafe {
+            self._release();
+        }
+        if !panicking() {
+            panic!("Reserved virtual memory wasn't released explicitly");
+        }
+    }
+}

src/winapi_wrappers.rs

@@ -1,8 +1,8 @@
-use std::cmp::max;
+use dynamic_memory_api_0::virtual_memory::ExtendResult;
 use std::ffi::c_void;
 use std::ptr::{NonNull, null_mut};
 use windows::Win32::Foundation::{
-    ERROR_INVALID_ADDRESS, ERROR_NOT_ENOUGH_MEMORY, GetLastError, WIN32_ERROR,
+    ERROR_INVALID_ADDRESS, ERROR_NOT_ENOUGH_MEMORY, GetLastError, WIN32_ERROR
 };
 use windows::Win32::System::Memory::{
     MEM_COMMIT, MEM_DECOMMIT, MEM_RELEASE, MEM_RESERVE, PAGE_NOACCESS, PAGE_READWRITE,
@@ -13,7 +13,7 @@ use windows::Win32::System::SystemInformation::{GetSystemInfo, SYSTEM_INFO};
 pub(super) unsafe fn reserve(
     start: Option<NonNull<c_void>>,
     size: usize,
-) -> Option<NonNull<c_void>> {
+) -> ExtendResult<NonNull<c_void>> {
     let raw = VirtualAlloc(
         start.map(|p| p.as_ptr() as *const c_void),
         size,
@@ -22,15 +22,13 @@ pub(super) unsafe fn reserve(
     );
     if raw == null_mut::<c_void>() {
         match GetLastError() {
-            ERROR_NOT_ENOUGH_MEMORY => return None,
+            ERROR_NOT_ENOUGH_MEMORY => return ExtendResult::OutOfMemory,
-            ERROR_INVALID_ADDRESS => {
+            ERROR_INVALID_ADDRESS => return ExtendResult::ContinuationIsBusy,
-                panic!("Requested address already in use (ERROR_INVALID_ADDRESS)")
-            }
             WIN32_ERROR(code) => panic!("Unexpected Win32 API Error: code={}", code),
         }
     };
 
-    return NonNull::new(raw);
+    return ExtendResult::Success(NonNull::new_unchecked(raw));
 }
 
 pub(super) unsafe fn commit(start: NonNull<c_void>, size: usize) {
@@ -63,8 +61,8 @@ pub(super) unsafe fn decommit(start: NonNull<c_void>, size: usize) {
     };
 }
 
-pub(super) unsafe fn release(start: NonNull<c_void>, size: usize) {
+pub(super) unsafe fn release(start: NonNull<c_void>) {
-    let result = VirtualFree(start.as_ptr(), size, MEM_RELEASE);
+    let result = VirtualFree(start.as_ptr(), 0, MEM_RELEASE);
 
     if result.is_err() {
         match GetLastError() {
@@ -78,10 +76,18 @@ pub(super) unsafe fn release(start: NonNull<c_void>, size: usize) {
     };
 }
 
-pub(super) fn getPageSize() -> usize {
+pub(super) struct SystemInfo {
+    pub pageSize: usize,
+    pub allocationGranularity: usize,
+}
+
+pub(super) fn getSystemInfo() -> SystemInfo {
     let mut si = SYSTEM_INFO::default();
     unsafe {
         GetSystemInfo(&mut si as *mut _);
     }
-    return max(si.dwPageSize, si.dwAllocationGranularity) as usize;
+    return SystemInfo {
+        pageSize: si.dwPageSize as usize,
+        allocationGranularity: si.dwAllocationGranularity as usize,
+    };
 }