Skip to content

Commit c4c2dcd

Browse files
authored
Fix the concurrency issue for munprotect for PageProtect (#401)
1 parent fde7c54 commit c4c2dcd

File tree

1 file changed

+13
-0
lines changed

1 file changed

+13
-0
lines changed

src/util/heap/freelistpageresource.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,19 @@ impl<VM: VMBinding> PageResource<VM> for FreeListPageResource<VM> {
119119
// The meta-data portion of reserved Pages was committed above.
120120
self.commit_pages(reserved_pages, required_pages, tls);
121121
if self.protect_memory_on_release && !new_chunk {
122+
use crate::util::heap::layout::Mmapper;
123+
use crate::MMAPPER;
124+
// This check is necessary to prevent us from mprotecting an address that is not yet mapped by mmapper.
125+
// See https://github.com/mmtk/mmtk-core/issues/400.
126+
// It is possible that one thread gets a new chunk, and returns from this function. However, the Space.acquire()
127+
// has not yet call ensure_mapped() for it. So the chunk is not yet mmapped. At this point, if another thread calls
128+
// this function, and get a few more pages from the same chunk, it is no longer seen as 'new_chunk', and we
129+
// will try to munprotect on it. But the chunk may not yet be mapped.
130+
//
131+
// If we want to improve and get rid of this loop, we need to move this munprotect to anywhere after the ensure_mapped() call
132+
// in Space.acquire(). We can either move it the option of 'protect_on_release' to space, or have a call to page resource
133+
// after ensure_mapped(). However, I think this is sufficient given that this option is only used for PageProtect for debugging use.
134+
while !MMAPPER.is_mapped_address(rtn) {}
122135
self.munprotect(rtn, self.free_list.size(page_offset as _) as _)
123136
};
124137
Result::Ok(PRAllocResult {

0 commit comments

Comments
 (0)