Remove cached RW mapping when the corresponding RX one is released

src/coreclr/inc/executableallocator.h

@@ -114,6 +114,9 @@ class ExecutableAllocator
     // and replaces it by the passed in one.
     void UpdateCachedMapping(BlockRW *pBlock);
 
+    // Remove the cached mapping
+    void RemoveCachedMapping();
+
     // Find existing RW block that maps the whole specified range of RX memory.
     // Return NULL if no such block exists.
     void* FindRWBlock(void* baseRX, size_t size);

src/coreclr/utilcode/executableallocator.cpp

@@ -261,28 +261,36 @@ bool ExecutableAllocator::Initialize()
 
 #define ENABLE_CACHED_MAPPINGS
 
-void ExecutableAllocator::UpdateCachedMapping(BlockRW* pBlock)
+void ExecutableAllocator::RemoveCachedMapping()
 {
-    LIMITED_METHOD_CONTRACT;
 #ifdef ENABLE_CACHED_MAPPINGS
-    if (m_cachedMapping == NULL)
+    void* unmapAddress = NULL;
+    size_t unmapSize;
+
+    if (!RemoveRWBlock(m_cachedMapping->baseRW, &unmapAddress, &unmapSize))
     {
-        m_cachedMapping = pBlock;
-        pBlock->refCount++;
+        g_fatalErrorHandler(COR_E_EXECUTIONENGINE, W("The RW block to unmap was not found"));
     }
-    else if (m_cachedMapping != pBlock)
+    if (unmapAddress && !VMToOSInterface::ReleaseRWMapping(unmapAddress, unmapSize))
     {
-        void* unmapAddress = NULL;
-        size_t unmapSize;
+        g_fatalErrorHandler(COR_E_EXECUTIONENGINE, W("Releasing the RW mapping failed"));
+    }
 
-        if (!RemoveRWBlock(m_cachedMapping->baseRW, &unmapAddress, &unmapSize))
-        {
-            g_fatalErrorHandler(COR_E_EXECUTIONENGINE, W("The RW block to unmap was not found"));
-        }
-        if (unmapAddress && !VMToOSInterface::ReleaseRWMapping(unmapAddress, unmapSize))
+    m_cachedMapping = NULL;
+#endif // ENABLE_CACHED_MAPPINGS
+}
+
+void ExecutableAllocator::UpdateCachedMapping(BlockRW* pBlock)
+{
+    LIMITED_METHOD_CONTRACT;
+#ifdef ENABLE_CACHED_MAPPINGS
+    if (m_cachedMapping != pBlock)
+    {
+        if (m_cachedMapping != NULL)
         {
-            g_fatalErrorHandler(COR_E_EXECUTIONENGINE, W("Releasing the RW mapping failed"));
+            RemoveCachedMapping();
         }
+
         m_cachedMapping = pBlock;
         pBlock->refCount++;
     }
@@ -453,6 +461,15 @@ void ExecutableAllocator::Release(void* pRX)
 
         if (pBlock != NULL)
         {
+            if (m_cachedMapping != NULL)
+            {
+                // In case the cached mapping maps the region being released, it needs to be removed
+                if ((pBlock->baseRX <= m_cachedMapping->baseRX) && (m_cachedMapping->baseRX < ((BYTE*)pBlock->baseRX + pBlock->size)))
+                {
+                    RemoveCachedMapping();
+                }
+            }
+
             if (!VMToOSInterface::ReleaseDoubleMappedMemory(m_doubleMemoryMapperHandle, pRX, pBlock->offset, pBlock->size))
             {
                 g_fatalErrorHandler(COR_E_EXECUTIONENGINE, W("Releasing the double mapped memory failed"));
@@ -467,6 +484,8 @@ void ExecutableAllocator::Release(void* pRX)
             // The block was not found, which should never happen.
             g_fatalErrorHandler(COR_E_EXECUTIONENGINE, W("The RX block to release was not found"));
         }
+
+        _ASSERTE(FindRWBlock(pRX, 1) == NULL);
     }
     else
     {
@@ -779,22 +798,22 @@ void* ExecutableAllocator::MapRW(void* pRX, size_t size)
     {
         if (pRX >= pBlock->baseRX && ((size_t)pRX + size) <= ((size_t)pBlock->baseRX + pBlock->size))
         {
-        // Offset of the RX address in the originally allocated block
-        size_t offset = (size_t)pRX - (size_t)pBlock->baseRX;
-        // Offset of the RX address that will start the newly mapped block
-        size_t mapOffset = ALIGN_DOWN(offset, Granularity());
-        // Size of the block we will map
-        size_t mapSize = ALIGN_UP(offset - mapOffset + size, Granularity());
+            // Offset of the RX address in the originally allocated block
+            size_t offset = (size_t)pRX - (size_t)pBlock->baseRX;
+            // Offset of the RX address that will start the newly mapped block
+            size_t mapOffset = ALIGN_DOWN(offset, Granularity());
+            // Size of the block we will map
+            size_t mapSize = ALIGN_UP(offset - mapOffset + size, Granularity());
 
 #ifdef LOG_EXECUTABLE_ALLOCATOR_STATISTICS
-        StopWatch sw2(&g_mapCreateTimeSum);
+            StopWatch sw2(&g_mapCreateTimeSum);
 #endif
-        void* pRW = VMToOSInterface::GetRWMapping(m_doubleMemoryMapperHandle, (BYTE*)pBlock->baseRX + mapOffset, pBlock->offset + mapOffset, mapSize);
+            void* pRW = VMToOSInterface::GetRWMapping(m_doubleMemoryMapperHandle, (BYTE*)pBlock->baseRX + mapOffset, pBlock->offset + mapOffset, mapSize);
 
-        if (pRW == NULL)
-        {
-            g_fatalErrorHandler(COR_E_EXECUTIONENGINE, W("Failed to create RW mapping for RX memory"));
-        }
+            if (pRW == NULL)
+            {
+                g_fatalErrorHandler(COR_E_EXECUTIONENGINE, W("Failed to create RW mapping for RX memory"));
+            }
 
             AddRWBlock(pRW, (BYTE*)pBlock->baseRX + mapOffset, mapSize);
 

src/coreclr/utilcode/loaderheap.cpp

@@ -1198,6 +1198,12 @@ BOOL UnlockedLoaderHeap::UnlockedReservePages(size_t dwSizeToCommit)
         return FALSE;
     }
 
+    NewHolder<LoaderHeapBlock> pNewBlock = new (nothrow) LoaderHeapBlock;
+    if (pNewBlock == NULL)
+    {
+        return FALSE;
+    }
+
     // Record reserved range in range list, if one is specified
     // Do this AFTER the commit - otherwise we'll have bogus ranges included.
     if (m_pRangeList != NULL)
@@ -1210,14 +1216,9 @@ BOOL UnlockedLoaderHeap::UnlockedReservePages(size_t dwSizeToCommit)
         }
     }
 
-    LoaderHeapBlock *pNewBlock = new (nothrow) LoaderHeapBlock;
-    if (pNewBlock == NULL)
-    {
-        return FALSE;
-    }
-
     m_dwTotalAlloc += dwSizeToCommit;
 
+    pNewBlock.SuppressRelease();
     pData.SuppressRelease();
 
     pNewBlock->dwVirtualSize    = dwSizeToReserve;