Hello,
Alvaro Herrera <[email protected]> wrote:
> Tsutomu Yamada wrote:
>
> > This patch using VirtualAlloc()/VirtualFree() to avoid failing in
> > reattach to shared memory.
> >
> > Can this be added to CommitFest ?
>
> Since this fixes a very annoying bug present in older versions, I think
> this should be backpatched all the way back to 8.2.
>
> Some notes about the patch itself:
>
> - please use ereport() instead of elog() for error messages
> - Are you really putting the pgwin32_ReserveSharedMemory declaration
> inside a function? Please move that into the appropriate header file.
> - Failure to reserve memory in pgwin32_ReserveSharedMemory should be a
> FATAL error I think, not simply LOG.
In this case,
the parent process operates child's memory by using VirtualAlloc().
If VirtualAlloc failed and be a FATAL error, master process will be stopped.
I think that is not preferable.
So, when VirtualAlloc failed, parent reports error and terminates child.
Revised patch
- move function declaration to include/port/win32.h
- add error check.
when VirtualAlloc failed, parent will terminate child process.
Thanks.
--
Tsutomu Yamada
SRA OSS, Inc. Japan
Index: src/backend/port/win32_shmem.c
===================================================================
RCS file: /mnt/prj/pg/cvsmirror/pg/pgsql/src/backend/port/win32_shmem.c,v
retrieving revision 1.11
diff -c -r1.11 win32_shmem.c
*** src/backend/port/win32_shmem.c 11 Jun 2009 14:49:00 -0000 1.11
--- src/backend/port/win32_shmem.c 15 Jul 2009 08:56:34 -0000
***************
*** 18,23 ****
--- 18,24 ----
unsigned long UsedShmemSegID = 0;
void *UsedShmemSegAddr = NULL;
+ static Size UsedShmemSegSize = 0;
static void pgwin32_SharedMemoryDelete(int status, Datum shmId);
***************
*** 233,238 ****
--- 234,240 ----
/* Save info for possible future use */
UsedShmemSegAddr = memAddress;
+ UsedShmemSegSize = size;
UsedShmemSegID = (unsigned long) hmap2;
return hdr;
***************
*** 257,262 ****
--- 259,273 ----
Assert(UsedShmemSegAddr != NULL);
Assert(IsUnderPostmaster);
+ /* release memory region
+ * that reserved by parant process
+ */
+ if (VirtualFree(UsedShmemSegAddr, 0, MEM_RELEASE) == 0)
+ {
+ elog(LOG, "failed to release reserved memory region (addr=%p):
%lu",
+ UsedShmemSegAddr, GetLastError());
+ }
+
hdr = (PGShmemHeader *) MapViewOfFileEx((HANDLE) UsedShmemSegID,
FILE_MAP_READ | FILE_MAP_WRITE, 0, 0, 0, UsedShmemSegAddr);
if (!hdr)
elog(FATAL, "could not reattach to shared memory (key=%d,
addr=%p): %lu",
***************
*** 302,304 ****
--- 313,337 ----
if (!CloseHandle((HANDLE) DatumGetInt32(shmId)))
elog(LOG, "could not close handle to shared memory: %lu",
GetLastError());
}
+
+ /*
+ * pgwin32_ReserveSharedMemory(HANDLE pChild)
+ * Reserve shared memory area,
+ * BEFORE child process allocates memory for DLL and/or others.
+ */
+ int
+ pgwin32_ReserveSharedMemory(HANDLE pChild)
+ {
+ void *memAddress;
+
+ Assert(UsedShmemSegAddr != NULL);
+ Assert(UsedShmemSegSize != 0);
+ memAddress = VirtualAllocEx(pChild, UsedShmemSegAddr, UsedShmemSegSize,
+ MEM_RESERVE,
PAGE_READWRITE);
+ if (memAddress == NULL) {
+ elog(LOG, "could not reserve shared memory region (addr=%p):
%lu",
+ UsedShmemSegAddr, GetLastError());
+ return false;
+ }
+ return true;
+ }
Index: src/backend/postmaster/postmaster.c
===================================================================
RCS file: /mnt/prj/pg/cvsmirror/pg/pgsql/src/backend/postmaster/postmaster.c,v
retrieving revision 1.584
diff -c -r1.584 postmaster.c
*** src/backend/postmaster/postmaster.c 8 Jul 2009 18:55:35 -0000 1.584
--- src/backend/postmaster/postmaster.c 15 Jul 2009 07:37:09 -0000
***************
*** 3643,3648 ****
--- 3643,3660 ----
elog(LOG, "could not close handle to backend parameter file:
error code %d",
(int) GetLastError());
+ /* reserve shared memory area before ResumeThread() */
+ if (!pgwin32_ReserveSharedMemory(pi.hProcess))
+ {
+ if (!TerminateProcess(pi.hProcess, 255))
+ ereport(ERROR,
+ (errmsg_internal("could not terminate
process that failed to reserve memory: error code %d",
+ (int)
GetLastError())));
+ CloseHandle(pi.hProcess);
+ CloseHandle(pi.hThread);
+ return -1; /* elog() made by
pgwin32_ReserveSharedMemory() */
+ }
+
/*
* Now that the backend variables are written out, we start the child
* thread so it can start initializing while we set up the rest of the
Index: src/include/port/win32.h
===================================================================
RCS file: /mnt/prj/pg/cvsmirror/pg/pgsql/src/include/port/win32.h,v
retrieving revision 1.88
diff -c -r1.88 win32.h
*** src/include/port/win32.h 11 Jun 2009 14:49:12 -0000 1.88
--- src/include/port/win32.h 15 Jul 2009 08:39:23 -0000
***************
*** 288,293 ****
--- 288,296 ----
extern int pgwin32_is_service(void);
#endif
+ /* in backend/port/win32_shmem.c */
+ extern int pgwin32_ReserveSharedMemory(HANDLE);
+
/* in port/win32error.c */
extern void _dosmaperr(unsigned long);
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
