Skip to content

Commit 24703d8

Browse files
ctruedenclaude
andcommitted
Remove incorrect VirtualAllocEx usage
Before: Called VirtualAllocEx on the pointer from MapViewOfFile, then unmapped both pointers. After: Use only MapViewOfFile pointer directly. Why this was wrong: - VirtualAllocEx is for allocating memory in other processes, not the current one - MapViewOfFile already returns a committed, writable pointer - Cleanup was using UnmapViewOfFile on both pointers, but should use VirtualFreeEx for the VirtualAllocEx pointer - This could cause subtle memory visibility issues on Windows Changes in ShmWindows.java: - Removed lines 118-128 (the VirtualAllocEx call and error handling) - Removed writePointer from ShmInfo struct assignment - Updated cleanup() to take only pointer and handle (removed writePointer parameter) Changes in ShmBase.java: - Removed writePointer field from ShmInfo<HANDLE> class - Removed writePointer from toString() method Co-authored-by: Claude <[email protected]>
1 parent 09a1406 commit 24703d8

File tree

2 files changed

+5
-18
lines changed

2 files changed

+5
-18
lines changed

src/main/java/org/apposed/appose/shm/ShmBase.java

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,6 @@ public String toString() {
106106
sb.append(getClass().getSimpleName()).append("{");
107107
sb.append("name='").append(name()).append("', size=").append(size());
108108
if (info.pointer != null) sb.append(", pointer=").append(info.pointer);
109-
if (info.writePointer != null) sb.append(", writePointer=").append(info.writePointer);
110109
if (info.handle != null) sb.append("handle=").append(info.handle);
111110
sb.append(", closed=").append(closed);
112111
sb.append(", unlinked=").append(unlinked);
@@ -129,8 +128,6 @@ protected static class ShmInfo<HANDLE> {
129128
/** Pointer referencing the shared memory. */
130129
Pointer pointer;
131130

132-
Pointer writePointer;
133-
134131
/** File handle for the shared memory block's (pseudo-)file. */
135132
HANDLE handle;
136133

src/main/java/org/apposed/appose/shm/ShmWindows.java

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ private SharedMemoryWindows(final String name, final boolean create, final long
6464

6565
@Override
6666
protected void doClose() {
67-
cleanup(info.pointer, info.writePointer, info.handle);
67+
cleanup(info.pointer, info.handle);
6868
}
6969

7070
@Override
@@ -115,24 +115,15 @@ private static ShmInfo<WinNT.HANDLE> prepareShm(String name, boolean create, lon
115115
throw new RuntimeException("Error mapping shared memory: " + lastError());
116116
}
117117

118-
Pointer writePointer = Kernel32.INSTANCE.VirtualAllocEx(
119-
Kernel32.INSTANCE.GetCurrentProcess(),
120-
pointer,
121-
new BaseTSD.SIZE_T(rsize),
122-
WinNT.MEM_COMMIT,
123-
WinNT.PAGE_READWRITE
124-
);
125-
if (isNull(writePointer)) {
126-
cleanup(pointer, writePointer, hMapFile);
127-
throw new RuntimeException("Error committing to the shared memory pages: " + lastError());
128-
}
118+
// Note: MapViewOfFile already returns a writable pointer.
119+
// We do not need VirtualAllocEx, which is for allocating memory in other processes.
120+
// The mapped view is already committed and accessible.
129121

130122
ShmInfo<WinNT.HANDLE> info = new ShmInfo<>();
131123
info.name = shmName;
132124
info.rsize = rsize; // REQUESTED size
133125
info.size = shm_size; // ALLOCATED size
134126
info.pointer = pointer;
135-
info.writePointer = writePointer;
136127
info.handle = hMapFile;
137128
info.unlinkOnClose = create;
138129
return info;
@@ -192,8 +183,7 @@ private static long getSHMSize(final WinNT.HANDLE hMapFile) {
192183
}
193184
}
194185

195-
private static void cleanup(Pointer pointer, Pointer writePointer, WinNT.HANDLE handle) {
196-
if (!isNull(writePointer)) Kernel32.INSTANCE.UnmapViewOfFile(writePointer);
186+
private static void cleanup(Pointer pointer, WinNT.HANDLE handle) {
197187
if (!isNull(pointer)) Kernel32.INSTANCE.UnmapViewOfFile(pointer);
198188
if (handle != null) Kernel32.INSTANCE.CloseHandle(handle);
199189
}

0 commit comments

Comments
 (0)