crates/stdlib/src/mmap.rs (1)

849-861: as_bytes_mut returns a mutable slice for Named regardless of access mode.

Unlike MmapObj::Read which panics, the Named arm always returns a mutable slice. While current callers (try_writable, buffer protocol) check access mode before reaching this point, adding a debug assertion would strengthen the invariant:

                 #[cfg(windows)]
-                MmapObj::Named(named) => unsafe {
-                    core::slice::from_raw_parts_mut(named.view_ptr, named.len)
-                },
+                MmapObj::Named(named) => {
+                    debug_assert!(
+                        !matches!(self.access, AccessMode::Read),
+                        "mmap can't modify a readonly memory map."
+                    );
+                    unsafe { core::slice::from_raw_parts_mut(named.view_ptr, named.len) }
+                }

Note: self is not directly accessible in as_bytes_mut since it operates on the MmapObj match. If restructuring isn't feasible, adding a comment documenting the caller-enforced invariant would suffice.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/stdlib/src/mmap.rs` around lines 849 - 861, The as_bytes_mut method
currently returns a mutable slice for MmapObj::Named regardless of access mode;
update as_bytes_mut (the match arm for MmapObj::Named) to either add a debug
assertion that the named mapping is writable (assert the Named object's
access/flags indicate writable access) or, if that field isn't accessible here,
add a clear comment above the method and the MmapObj::Named arm documenting the
caller-enforced invariant (try_writable and buffer protocol callers must have
verified writability) so future readers/maintainers know why a mutable slice is
returned despite no runtime check; reference as_bytes_mut, MmapObj::Named,
MmapObj::Read, try_writable and the buffer protocol in the comment or assertion.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

crates/stdlib/src/mmap.rs (1)

671-680: ⚠️ Potential issue | 🟠 Major

File-backed named mappings lose the OS file handle — size() won't return the file size.

When a tagname is provided alongside a real file handle, the duplicated handle is closed and self.handle is set to INVALID_HANDLE_VALUE. This causes size() (line 1331) to return self.__len__() (the mapping length) instead of querying the actual file size via GetFileSize. CPython's mmap.size() returns the file size for file-backed mappings.

Consider storing duplicated_handle in self.handle for file-backed named mmaps instead of closing it, or add a comment explaining the intentional divergence from CPython behavior.

Also applies to: 720-729

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/stdlib/src/mmap.rs` around lines 671 - 680, The code closes
duplicated_handle and leaves self.handle as INVALID_HANDLE_VALUE for file-backed
named mappings, causing size() to fall back to the mapping length instead of
using GetFileSize; change the logic in the block that currently returns
fh/INVALID_HANDLE_VALUE (the code handling fh and duplicated_handle) so that for
file-backed named mmaps you keep the duplicated_handle (assign duplicated_handle
to self.handle) instead of closing it, and only close the original
duplicated_handle when it is truly unused; ensure the new ownership is
consistent with the destructor/cleanup so no handle is leaked and size() can
call GetFileSize on self.handle.

crates/stdlib/src/mmap.rs (1)

851-863: as_bytes_mut doesn't guard against Read-mode Named mappings.

MmapObj::Read panics here (line 854), but a Named mmap created with AccessMode::Read (FILE_MAP_READ) will silently return a mutable slice. Writing to that memory would cause an OS-level access violation. While callers currently check access mode upstream, adding a consistent guard here would be safer.

🛡️ Suggested defensive guard

                    #[cfg(windows)]
                    MmapObj::Named(named) => unsafe {
+                       debug_assert!(
+                           !matches!(
+                               *m.as_ref().unwrap(),
+                               MmapObj::Named(_) if false // access check is on PyMmap, not MmapObj
+                           )
+                       );
                        core::slice::from_raw_parts_mut(named.view_ptr, named.len)
                    },

Actually, since as_bytes_mut doesn't have access to self.access, a simpler approach: document that callers must check access mode, mirroring the MmapObj::Read panic's implicit contract.

Alternatively, pass access into the function or move the check to the match:

 fn as_bytes_mut(&self) -> BorrowedValueMut<'_, [u8]> {
+    assert!(
+        self.access != AccessMode::Read,
+        "mmap can't modify a readonly memory map."
+    );
     PyMutexGuard::map(self.mmap.lock(), |m| {
         match m.as_mut().expect("mmap closed or invalid") {
             MmapObj::Read(_) => panic!("mmap can't modify a readonly memory map."),
             MmapObj::Write(mmap) => &mut mmap[..],

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/stdlib/src/mmap.rs` around lines 851 - 863, as_bytes_mut currently
returns a mutable slice for MmapObj::Named even if that named mapping was opened
read-only, which can lead to an access violation; update as_bytes_mut (or its
caller) to check the mapping's access mode and refuse/mirror the MmapObj::Read
behavior for read-only Named maps: either accept and use an access parameter in
as_bytes_mut and panic when access == AccessMode::Read for MmapObj::Named, or
add a guard in the match arm for MmapObj::Named that inspects the named
mapping's access flag and panics if it's read-only, ensuring Named maps opened
with AccessMode::Read are not exposed as mutable slices.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro