Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
On Tue, Jul 20, 2010 at 17:31, Etienne Dube etd...@gmail.com wrote: On 09/02/2010 4:09 PM, Etienne Dube wrote: Magnus Hagander wrote: IIRC, we've had zero reports on whether the patch worked at all on 8.2 in an environment where the problem actually existed. So yes, some testing and feedback would be much apprecaited. //Magnus Thanks for your quick reply. We upgraded to Service Pack 2 and it solved the problem. Nevertheless, I'll try to reproduce the issue under a Win2008 SP1 VM to see whether the patch makes a difference. 8.2.x win32 binaries are built using MinGW right? Etienne The could not reattach to shared memory bug came back to bite us, this time on a production machine running Windows Server 2008 R2 x64. I manually applied the patch against the 8.2.17 sources and installed the build on a test server. It has been running for two days without any issue. We'll see after a while if the patch actually fixes the problem (it seems to happen only after the postgres service has been up and running for some time) but in case you want to include this fix in a future 8.2.18 release, I've attached the new patch to apply against the REL8_2_STABLE branch. Yes, I think it's time to backpatch this to 8.2 - it has worked very well on 8.3 and 8.4, and we've had a couple of good reports on 8.2 by now. So I've done that, so it should be in the next 8.2 version. In fact, there was a small bug in the patch that broke all non-win32 platforms, so I fixed that while at it :-) -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
On 09/02/2010 4:09 PM, Etienne Dube wrote:
Magnus Hagander wrote:
IIRC, we've had zero reports on whether the patch worked at all on 8.2
in an environment where the problem actually existed. So yes, some
testing and feedback would be much apprecaited.
//Magnus
Thanks for your quick reply.
We upgraded to Service Pack 2 and it solved the problem. Nevertheless,
I'll try to reproduce the issue under a Win2008 SP1 VM to see whether
the patch makes a difference. 8.2.x win32 binaries are built using
MinGW right?
Etienne
The could not reattach to shared memory bug came back to bite us, this
time on a production machine running Windows Server 2008 R2 x64. I
manually applied the patch against the 8.2.17 sources and installed the
build on a test server. It has been running for two days without any
issue. We'll see after a while if the patch actually fixes the problem
(it seems to happen only after the postgres service has been up and
running for some time) but in case you want to include this fix in a
future 8.2.18 release, I've attached the new patch to apply against the
REL8_2_STABLE branch.
Etienne
Index: backend/port/sysv_shmem.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/port/sysv_shmem.c,v
retrieving revision 1.47.2.2
diff -u -r1.47.2.2 sysv_shmem.c
--- backend/port/sysv_shmem.c 1 May 2010 22:46:50 - 1.47.2.2
+++ backend/port/sysv_shmem.c 17 Jul 2010 19:19:51 -
@@ -49,6 +49,10 @@
unsigned long UsedShmemSegID = 0;
void *UsedShmemSegAddr = NULL;
+#ifdef WIN32
+Size UsedShmemSegSize = 0;
+#endif
+
static void *InternalIpcMemoryCreate(IpcMemoryKey memKey, Size size);
static void IpcMemoryDetach(int status, Datum shmaddr);
@@ -445,6 +449,7 @@
/* Save info for possible future use */
UsedShmemSegAddr = memAddress;
+ UsedShmemSegSize = size;
UsedShmemSegID = (unsigned long) NextShmemSegID;
return hdr;
Index: backend/port/win32/shmem.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/port/win32/Attic/shmem.c,v
retrieving revision 1.13.2.1
diff -u -r1.13.2.1 shmem.c
--- backend/port/win32/shmem.c 4 May 2009 08:36:42 - 1.13.2.1
+++ backend/port/win32/shmem.c 17 Jul 2010 19:22:13 -
@@ -12,8 +12,11 @@
*/
#include postgres.h
+#include miscadmin.h
static DWORD s_segsize = 0;
+extern void *UsedShmemSegAddr;
+extern Size UsedShmemSegSize;
/* Detach from a shared mem area based on its address */
int
@@ -29,6 +32,13 @@
void *
shmat(int memId, void *shmaddr, int flag)
{
+ /* Release the memory region reserved in the postmaster */
+ if (IsUnderPostmaster)
+ {
+ if (VirtualFree(shmaddr, 0, MEM_RELEASE) == 0)
+ elog(FATAL, failed to release reserved memory region
(addr=%p): %lu,
+shmaddr, GetLastError());
+ }
/* TODO -- shmat needs to count # attached to shared mem */
void *lpmem = MapViewOfFileEx((HANDLE) memId,
FILE_MAP_WRITE | FILE_MAP_READ,
@@ -128,3 +138,53 @@
return (int) hmap;
}
+
+/*
+ * pgwin32_ReserveSharedMemoryRegion(hChild)
+ *
+ * Reserve the memory region that will be used for shared memory in a child
+ * process. It is called before the child process starts, to make sure the
+ * memory is available.
+ *
+ * Once the child starts, DLLs loading in different order or threads getting
+ * scheduled differently may allocate memory which can conflict with the
+ * address space we need for our shared memory. By reserving the shared
+ * memory region before the child starts, and freeing it only just before we
+ * attempt to get access to the shared memory forces these allocations to
+ * be given different address ranges that don't conflict.
+ *
+ * NOTE! This function executes in the postmaster, and should for this
+ * reason not use elog(FATAL) since that would take down the postmaster.
+ */
+int
+pgwin32_ReserveSharedMemoryRegion(HANDLE hChild)
+{
+ void *address;
+
+ Assert(UsedShmemSegAddr != NULL);
+ Assert(UsedShmemSegSize != 0);
+
+ address = VirtualAllocEx(hChild, UsedShmemSegAddr, UsedShmemSegSize,
+ MEM_RESERVE,
PAGE_READWRITE);
+ if (address == NULL) {
+ /* Don't use FATAL since we're running in the postmaster */
+ elog(LOG, could not reserve shared memory region (addr=%p) for
child %lu: %lu,
+UsedShmemSegAddr, hChild, GetLastError());
+ return false;
+ }
+ if (address != UsedShmemSegAddr)
+ {
+ /*
+* Should never happen - in theory if allocation granularity
causes strange
+* effects it could, so check just in case.
+
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
Magnus Hagander wrote: IIRC, we've had zero reports on whether the patch worked at all on 8.2 in an environment where the problem actually existed. So yes, some testing and feedback would be much apprecaited. //Magnus Thanks for your quick reply. We upgraded to Service Pack 2 and it solved the problem. Nevertheless, I'll try to reproduce the issue under a Win2008 SP1 VM to see whether the patch makes a difference. 8.2.x win32 binaries are built using MinGW right? Etienne 2010/2/8 Etienne Dube etd...@gmail.com: Hi, We've come across this issue on 8.2.15 on a Windows Server 2008 instance. I noticed the patch hasn't been applied to the 8.2 branch yet. Any chances that this will be part of an eventual 8.2.16 release? Do you need more testing and feedback before commiting the patch? Thanks, Etienne Dube * *From*: Magnus Hagander mag...@hagander.net * *To*: Tom Lane t...@sss.pgh.pa.us * *Cc*: Tsutomu Yamada tsut...@sraoss.co.jp, Alvaro Herrera alvhe...@commandprompt.com, pgsql-hackers@postgresql.org, Dave Page dp...@pgadmin.org * *Subject*: Re: [PATCH] could not reattach to shared memory on Windows * *Date*: Tue, 11 Aug 2009 17:14:08 +0200 * *Message-id*: 9837222c0908110814n414b2fcbxcaf7c0e1fcc05...@mail.gmail.com http://archives.postgresql.org/pgsql-hackers/2009-08/msg00894.php On Tue, Aug 11, 2009 at 16:30, Magnus Hagandermag...@hagander.net wrote: On Mon, Aug 10, 2009 at 19:33, Magnus Hagandermag...@hagander.net wrote: On Mon, Aug 10, 2009 at 16:58, Tom Lanet...@sss.pgh.pa.us wrote: Magnus Hagander mag...@hagander.net writes: On Mon, Aug 10, 2009 at 16:10, Tom Lanet...@sss.pgh.pa.us wrote: 8.2 as well, no? 8.2 has a different shmem implementation - the one that emulates sysv shmem. The patch will need to be changed around for that, and I haven't looked at that. It may be worthwhile to do that, but it's a separate patch, so let's get it out in 8.3 and 8.4 first. If it's at all hard to do, I could see deprecating 8.2 for Windows instead. I haven't looked at how much work it would be at all yet. So let's do that before we decide to deprecate anything. As mentioned downthread, 8.2 is a very widespread release, and we really want to avoid deprecating it. Here's an attempt at a backport to 8.2. I haven't examined it in detail, but it passes make check on mingw. Comments? I've also built a binary that should be copy:able on top of an 8.2.13 installation made from the standard installer, to test this feature. Anybody on 8.2 on Windows, please give it a shot and let us know how it works. http://www.hagander.net/pgsql/postgres_exe_virtualalloc_8_2.zip -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
Hi, We've come across this issue on 8.2.15 on a Windows Server 2008 instance. I noticed the patch hasn't been applied to the 8.2 branch yet. Any chances that this will be part of an eventual 8.2.16 release? Do you need more testing and feedback before commiting the patch? Thanks, Etienne Dube * *From*: Magnus Hagander mag...@hagander.net * *To*: Tom Lane t...@sss.pgh.pa.us * *Cc*: Tsutomu Yamada tsut...@sraoss.co.jp, Alvaro Herrera alvhe...@commandprompt.com, pgsql-hackers@postgresql.org, Dave Page dp...@pgadmin.org * *Subject*: Re: [PATCH] could not reattach to shared memory on Windows * *Date*: Tue, 11 Aug 2009 17:14:08 +0200 * *Message-id*: 9837222c0908110814n414b2fcbxcaf7c0e1fcc05...@mail.gmail.com http://archives.postgresql.org/pgsql-hackers/2009-08/msg00894.php On Tue, Aug 11, 2009 at 16:30, Magnus Hagandermag...@hagander.net wrote: On Mon, Aug 10, 2009 at 19:33, Magnus Hagandermag...@hagander.net wrote: On Mon, Aug 10, 2009 at 16:58, Tom Lanet...@sss.pgh.pa.us wrote: Magnus Hagander mag...@hagander.net writes: On Mon, Aug 10, 2009 at 16:10, Tom Lanet...@sss.pgh.pa.us wrote: 8.2 as well, no? 8.2 has a different shmem implementation - the one that emulates sysv shmem. The patch will need to be changed around for that, and I haven't looked at that. It may be worthwhile to do that, but it's a separate patch, so let's get it out in 8.3 and 8.4 first. If it's at all hard to do, I could see deprecating 8.2 for Windows instead. I haven't looked at how much work it would be at all yet. So let's do that before we decide to deprecate anything. As mentioned downthread, 8.2 is a very widespread release, and we really want to avoid deprecating it. Here's an attempt at a backport to 8.2. I haven't examined it in detail, but it passes make check on mingw. Comments? I've also built a binary that should be copy:able on top of an 8.2.13 installation made from the standard installer, to test this feature. Anybody on 8.2 on Windows, please give it a shot and let us know how it works. http://www.hagander.net/pgsql/postgres_exe_virtualalloc_8_2.zip -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
IIRC, we've had zero reports on whether the patch worked at all on 8.2 in an environment where the problem actually existed. So yes, some testing and feedback would be much apprecaited. //Magnus 2010/2/8 Etienne Dube etd...@gmail.com: Hi, We've come across this issue on 8.2.15 on a Windows Server 2008 instance. I noticed the patch hasn't been applied to the 8.2 branch yet. Any chances that this will be part of an eventual 8.2.16 release? Do you need more testing and feedback before commiting the patch? Thanks, Etienne Dube * *From*: Magnus Hagander mag...@hagander.net * *To*: Tom Lane t...@sss.pgh.pa.us * *Cc*: Tsutomu Yamada tsut...@sraoss.co.jp, Alvaro Herrera alvhe...@commandprompt.com, pgsql-hackers@postgresql.org, Dave Page dp...@pgadmin.org * *Subject*: Re: [PATCH] could not reattach to shared memory on Windows * *Date*: Tue, 11 Aug 2009 17:14:08 +0200 * *Message-id*: 9837222c0908110814n414b2fcbxcaf7c0e1fcc05...@mail.gmail.com http://archives.postgresql.org/pgsql-hackers/2009-08/msg00894.php On Tue, Aug 11, 2009 at 16:30, Magnus Hagandermag...@hagander.net wrote: On Mon, Aug 10, 2009 at 19:33, Magnus Hagandermag...@hagander.net wrote: On Mon, Aug 10, 2009 at 16:58, Tom Lanet...@sss.pgh.pa.us wrote: Magnus Hagander mag...@hagander.net writes: On Mon, Aug 10, 2009 at 16:10, Tom Lanet...@sss.pgh.pa.us wrote: 8.2 as well, no? 8.2 has a different shmem implementation - the one that emulates sysv shmem. The patch will need to be changed around for that, and I haven't looked at that. It may be worthwhile to do that, but it's a separate patch, so let's get it out in 8.3 and 8.4 first. If it's at all hard to do, I could see deprecating 8.2 for Windows instead. I haven't looked at how much work it would be at all yet. So let's do that before we decide to deprecate anything. As mentioned downthread, 8.2 is a very widespread release, and we really want to avoid deprecating it. Here's an attempt at a backport to 8.2. I haven't examined it in detail, but it passes make check on mingw. Comments? I've also built a binary that should be copy:able on top of an 8.2.13 installation made from the standard installer, to test this feature. Anybody on 8.2 on Windows, please give it a shot and let us know how it works. http://www.hagander.net/pgsql/postgres_exe_virtualalloc_8_2.zip -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
On Mon, Aug 10, 2009 at 13:41, Magnus Hagandermag...@hagander.net wrote: On Wed, Jul 22, 2009 at 17:05, Magnus Hagandermag...@hagander.net wrote: Dave has built binaries for 8.3.7 and 8.4.0 for this, available at: http://developer.pgadmin.org/~dpage/postgres_exe_virtualalloc-8_3.zip http://developer.pgadmin.org/~dpage/postgres_exe_virtualalloc-8_4.zip We would like as many people as possible to test this both on systems that currently experience the problem and systems that don't, and let us know the status. To test, just replace your current postgres.exe binary with the one in the appropriate ZIP file above. Obviously, take a backup before you do it! These binaries contain just this one patch - the rest of what's been applied to the 8.3 and 8.4 branches for the next minor version is *not* included. It's been a couple of weeks now, and I've had a number of reports both on-list, on-blog and in private, from people using this. I have not yet had a single report of a problem caused by this patch (not counting the case where there was a version mismatch - can't fault the patch for that). Given that, I say we apply this for 8.3 and 8.4 now. Comments? Backpatched to 8.3 and 8.4 for now. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
On Mon, Aug 10, 2009 at 19:33, Magnus Hagandermag...@hagander.net wrote:
On Mon, Aug 10, 2009 at 16:58, Tom Lanet...@sss.pgh.pa.us wrote:
Magnus Hagander mag...@hagander.net writes:
On Mon, Aug 10, 2009 at 16:10, Tom Lanet...@sss.pgh.pa.us wrote:
8.2 as well, no?
8.2 has a different shmem implementation - the one that emulates sysv
shmem. The patch will need to be changed around for that, and I
haven't looked at that. It may be worthwhile to do that, but it's a
separate patch, so let's get it out in 8.3 and 8.4 first.
If it's at all hard to do, I could see deprecating 8.2 for Windows
instead.
I haven't looked at how much work it would be at all yet. So let's do
that before we decide to deprecate anything. As mentioned downthread,
8.2 is a very widespread release, and we really want to avoid
deprecating it.
Here's an attempt at a backport to 8.2. I haven't examined it in
detail, but it passes make check on mingw.
Comments?
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
*** a/src/backend/port/sysv_shmem.c
--- b/src/backend/port/sysv_shmem.c
***
*** 49,54 typedef int IpcMemoryId; /* shared memory ID returned by shmget(2) */
--- 49,57
unsigned long UsedShmemSegID = 0;
void *UsedShmemSegAddr = NULL;
+ #ifdef WIN32
+ Size UsedShmemSegSize = 0;
+ #endif
static void *InternalIpcMemoryCreate(IpcMemoryKey memKey, Size size);
static void IpcMemoryDetach(int status, Datum shmaddr);
***
*** 403,408 PGSharedMemoryCreate(Size size, bool makePrivate, int port)
--- 406,412
/* Save info for possible future use */
UsedShmemSegAddr = memAddress;
+ UsedShmemSegSize = size;
UsedShmemSegID = (unsigned long) NextShmemSegID;
return hdr;
*** a/src/backend/port/win32/shmem.c
--- b/src/backend/port/win32/shmem.c
***
*** 12,19
--- 12,22
*/
#include postgres.h
+ #include miscadmin.h
static DWORD s_segsize = 0;
+ extern void *UsedShmemSegAddr;
+ extern Size UsedShmemSegSize;
/* Detach from a shared mem area based on its address */
int
***
*** 29,34 shmdt(const void *shmaddr)
--- 32,44
void *
shmat(int memId, void *shmaddr, int flag)
{
+ /* Release the memory region reserved in the postmaster */
+ if (IsUnderPostmaster)
+ {
+ if (VirtualFree(shmaddr, 0, MEM_RELEASE) == 0)
+ elog(FATAL, failed to release reserved memory region (addr=%p): %lu,
+ shmaddr, GetLastError());
+ }
/* TODO -- shmat needs to count # attached to shared mem */
void *lpmem = MapViewOfFileEx((HANDLE) memId,
FILE_MAP_WRITE | FILE_MAP_READ,
***
*** 128,130 shmget(int memKey, int size, int flag)
--- 138,190
return (int) hmap;
}
+
+ /*
+ * pgwin32_ReserveSharedMemoryRegion(hChild)
+ *
+ * Reserve the memory region that will be used for shared memory in a child
+ * process. It is called before the child process starts, to make sure the
+ * memory is available.
+ *
+ * Once the child starts, DLLs loading in different order or threads getting
+ * scheduled differently may allocate memory which can conflict with the
+ * address space we need for our shared memory. By reserving the shared
+ * memory region before the child starts, and freeing it only just before we
+ * attempt to get access to the shared memory forces these allocations to
+ * be given different address ranges that don't conflict.
+ *
+ * NOTE! This function executes in the postmaster, and should for this
+ * reason not use elog(FATAL) since that would take down the postmaster.
+ */
+ int
+ pgwin32_ReserveSharedMemoryRegion(HANDLE hChild)
+ {
+ void *address;
+
+ Assert(UsedShmemSegAddr != NULL);
+ Assert(UsedShmemSegSize != 0);
+
+ address = VirtualAllocEx(hChild, UsedShmemSegAddr, UsedShmemSegSize,
+ MEM_RESERVE, PAGE_READWRITE);
+ if (address == NULL) {
+ /* Don't use FATAL since we're running in the postmaster */
+ elog(LOG, could not reserve shared memory region (addr=%p) for child %lu: %lu,
+ UsedShmemSegAddr, hChild, GetLastError());
+ return false;
+ }
+ if (address != UsedShmemSegAddr)
+ {
+ /*
+ * Should never happen - in theory if allocation granularity causes strange
+ * effects it could, so check just in case.
+ *
+ * Don't use FATAL since we're running in the postmaster.
+ */
+ elog(LOG, reserved shared memory region got incorrect address %p, expected %p,
+ address, UsedShmemSegAddr);
+ VirtualFreeEx(hChild, address, 0, MEM_RELEASE);
+ return false;
+ }
+
+ return true;
+ }
*** a/src/backend/postmaster/postmaster.c
--- b/src/backend/postmaster/postmaster.c
***
*** 3184,3190 internal_forkexec(int argc, char *argv[], Port *port)
return -1;/* log made by save_backend_variables */
}
! /* Drop the shared memory that is now inherited to the backend */
if (!UnmapViewOfFile(param))
elog(LOG, could not unmap view of
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
On Tue, Aug 11, 2009 at 16:30, Magnus Hagandermag...@hagander.net wrote: On Mon, Aug 10, 2009 at 19:33, Magnus Hagandermag...@hagander.net wrote: On Mon, Aug 10, 2009 at 16:58, Tom Lanet...@sss.pgh.pa.us wrote: Magnus Hagander mag...@hagander.net writes: On Mon, Aug 10, 2009 at 16:10, Tom Lanet...@sss.pgh.pa.us wrote: 8.2 as well, no? 8.2 has a different shmem implementation - the one that emulates sysv shmem. The patch will need to be changed around for that, and I haven't looked at that. It may be worthwhile to do that, but it's a separate patch, so let's get it out in 8.3 and 8.4 first. If it's at all hard to do, I could see deprecating 8.2 for Windows instead. I haven't looked at how much work it would be at all yet. So let's do that before we decide to deprecate anything. As mentioned downthread, 8.2 is a very widespread release, and we really want to avoid deprecating it. Here's an attempt at a backport to 8.2. I haven't examined it in detail, but it passes make check on mingw. Comments? I've also built a binary that should be copy:able on top of an 8.2.13 installation made from the standard installer, to test this feature. Anybody on 8.2 on Windows, please give it a shot and let us know how it works. http://www.hagander.net/pgsql/postgres_exe_virtualalloc_8_2.zip -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
On Wed, Jul 22, 2009 at 17:05, Magnus Hagandermag...@hagander.net wrote: Dave has built binaries for 8.3.7 and 8.4.0 for this, available at: http://developer.pgadmin.org/~dpage/postgres_exe_virtualalloc-8_3.zip http://developer.pgadmin.org/~dpage/postgres_exe_virtualalloc-8_4.zip We would like as many people as possible to test this both on systems that currently experience the problem and systems that don't, and let us know the status. To test, just replace your current postgres.exe binary with the one in the appropriate ZIP file above. Obviously, take a backup before you do it! These binaries contain just this one patch - the rest of what's been applied to the 8.3 and 8.4 branches for the next minor version is *not* included. It's been a couple of weeks now, and I've had a number of reports both on-list, on-blog and in private, from people using this. I have not yet had a single report of a problem caused by this patch (not counting the case where there was a version mismatch - can't fault the patch for that). Given that, I say we apply this for 8.3 and 8.4 now. Comments? -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
Magnus Hagander mag...@hagander.net writes: It's been a couple of weeks now, and I've had a number of reports both on-list, on-blog and in private, from people using this. I have not yet had a single report of a problem caused by this patch (not counting the case where there was a version mismatch - can't fault the patch for that). Given that, I say we apply this for 8.3 and 8.4 now. Comments? 8.2 as well, no? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
On Mon, Aug 10, 2009 at 16:10, Tom Lanet...@sss.pgh.pa.us wrote: Magnus Hagander mag...@hagander.net writes: It's been a couple of weeks now, and I've had a number of reports both on-list, on-blog and in private, from people using this. I have not yet had a single report of a problem caused by this patch (not counting the case where there was a version mismatch - can't fault the patch for that). Given that, I say we apply this for 8.3 and 8.4 now. Comments? 8.2 as well, no? 8.2 has a different shmem implementation - the one that emulates sysv shmem. The patch will need to be changed around for that, and I haven't looked at that. It may be worthwhile to do that, but it's a separate patch, so let's get it out in 8.3 and 8.4 first. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
On Mon, Aug 10, 2009 at 3:33 PM, Magnus Hagandermag...@hagander.net wrote: On Mon, Aug 10, 2009 at 16:10, Tom Lanet...@sss.pgh.pa.us wrote: Magnus Hagander mag...@hagander.net writes: It's been a couple of weeks now, and I've had a number of reports both on-list, on-blog and in private, from people using this. I have not yet had a single report of a problem caused by this patch (not counting the case where there was a version mismatch - can't fault the patch for that). Given that, I say we apply this for 8.3 and 8.4 now. Comments? 8.2 as well, no? 8.2 has a different shmem implementation - the one that emulates sysv shmem. The patch will need to be changed around for that, and I haven't looked at that. It may be worthwhile to do that, but it's a separate patch, so let's get it out in 8.3 and 8.4 first. Has anyone reported the problem on 8.2? -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
On Mon, Aug 10, 2009 at 16:45, Dave Pagedp...@pgadmin.org wrote: On Mon, Aug 10, 2009 at 3:33 PM, Magnus Hagandermag...@hagander.net wrote: On Mon, Aug 10, 2009 at 16:10, Tom Lanet...@sss.pgh.pa.us wrote: Magnus Hagander mag...@hagander.net writes: It's been a couple of weeks now, and I've had a number of reports both on-list, on-blog and in private, from people using this. I have not yet had a single report of a problem caused by this patch (not counting the case where there was a version mismatch - can't fault the patch for that). Given that, I say we apply this for 8.3 and 8.4 now. Comments? 8.2 as well, no? 8.2 has a different shmem implementation - the one that emulates sysv shmem. The patch will need to be changed around for that, and I haven't looked at that. It may be worthwhile to do that, but it's a separate patch, so let's get it out in 8.3 and 8.4 first. Has anyone reported the problem on 8.2? Yes. I've seen reports of it all the way back to 8.0. It does seem to have increased in frequently with Win2003 and Win2008 as the server platforms, which means the newer versions have had a higher percentage, but the issue definitely exists. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
On Mon, Aug 10, 2009 at 3:49 PM, Magnus Hagandermag...@hagander.net wrote: Has anyone reported the problem on 8.2? Yes. I've seen reports of it all the way back to 8.0. It does seem to have increased in frequently with Win2003 and Win2008 as the server platforms, which means the newer versions have had a higher percentage, but the issue definitely exists. I suppose there's some question of whether this is the kind of issue we need to bother supporting for back-branches. The whole point of supporting back branches is so that people who are already using them can expect to have any known problems they might run into fixed. If people are still running these old branches then presumably their setup isn't prone to this problem. If they're going to update to Win2003 or Win2008 then that's a whole new installation, not an existing installation which might suddenly run into this problem. Is the reason we support old branches so that people can install those old branches in preference to newer ones? Or just so that people who have already installed them can continue to rely on them? The flaws in this line of argument are that a) I'm not entirely sure my premise that someone who has been running fine won't suddenly run into this problem is true. And b) nor am I entirely clear that you have to reinstall Postgres or other apps when you upgrade Windows. -- greg http://mit.edu/~gsstark/resume.pdf -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
Magnus Hagander mag...@hagander.net writes: On Mon, Aug 10, 2009 at 16:10, Tom Lanet...@sss.pgh.pa.us wrote: 8.2 as well, no? 8.2 has a different shmem implementation - the one that emulates sysv shmem. The patch will need to be changed around for that, and I haven't looked at that. It may be worthwhile to do that, but it's a separate patch, so let's get it out in 8.3 and 8.4 first. If it's at all hard to do, I could see deprecating 8.2 for Windows instead. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
On Mon, Aug 10, 2009 at 3:58 PM, Tom Lanet...@sss.pgh.pa.us wrote: Magnus Hagander mag...@hagander.net writes: On Mon, Aug 10, 2009 at 16:10, Tom Lanet...@sss.pgh.pa.us wrote: 8.2 as well, no? 8.2 has a different shmem implementation - the one that emulates sysv shmem. The patch will need to be changed around for that, and I haven't looked at that. It may be worthwhile to do that, but it's a - separate patch, so let's get it out in 8.3 and 8.4 first. If it's at all hard to do, I could see deprecating 8.2 for Windows instead. I could most definitely agree with that on a personal level - no more Mingw/msys builds to maintain :-) Alas, it's probably not practical to drop it without inconveniencing a great many Windows users. -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
Dave Page wrote: If it's at all hard to do, I could see deprecating 8.2 for Windows instead. I could most definitely agree with that on a personal level - no more Mingw/msys builds to maintain :-) Alas, it's probably not practical to drop it without inconveniencing a great many Windows users. I hope you're not suggesting we drop Mingw/MSys as a build platform, even if you personally don't want to build with it. I would have found it much harder to do parallel restore for Windows (which works quite differently from Unix, and so had to be specifically developed) if I had been forced to use the MS tool set with which I don't ever otherwise work. I don't think we should deprecate 8.2 on Windows unless we really can't backport this fix reasonably. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
On Mon, Aug 10, 2009 at 4:29 PM, Andrew Dunstanand...@dunslane.net wrote: I hope you're not suggesting we drop Mingw/MSys as a build platform, even if you personally don't want to build with it. I would have found it much harder to do parallel restore for Windows (which works quite differently from Unix, and so had to be specifically developed) if I had been forced to use the MS tool set with which I don't ever otherwise work. Not at all - in fact we need it to maintain some of the other apps like PostGIS or Slony. I'm just talking about my own use of it for building PG release builds. I don't think we should deprecate 8.2 on Windows unless we really can't backport this fix reasonably. Agreed. There are too many users, and it wouldn't be fair to them. -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
On Mon, Aug 10, 2009 at 16:58, Tom Lanet...@sss.pgh.pa.us wrote: Magnus Hagander mag...@hagander.net writes: On Mon, Aug 10, 2009 at 16:10, Tom Lanet...@sss.pgh.pa.us wrote: 8.2 as well, no? 8.2 has a different shmem implementation - the one that emulates sysv shmem. The patch will need to be changed around for that, and I haven't looked at that. It may be worthwhile to do that, but it's a separate patch, so let's get it out in 8.3 and 8.4 first. If it's at all hard to do, I could see deprecating 8.2 for Windows instead. I haven't looked at how much work it would be at all yet. So let's do that before we decide to deprecate anything. As mentioned downthread, 8.2 is a very widespread release, and we really want to avoid deprecating it. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
On Thu, Jul 23, 2009 at 09:04, Magnus Hagandermag...@hagander.net wrote: On Thu, Jul 23, 2009 at 08:04, Tsutomu Yamadatsut...@sraoss.co.jp wrote: Hello, Thank you for correcting patch. However, I think the following block have to use VirualFree*Ex*(). (yes, this should never happen, maybe there is actually no problem. but for logical correctness) That is definitely correct. I have updated the patch in my tree and will make sure to include that in the eventual commit. FYI, and others, I have received a couple of off-list reports from people testing out the patch, and so far only positive results. I have applied this patch to HEAD so we can get buildfarm coverage. Holding back on the batckpatch for a bit longer. -- Magnus Hagander Self: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
Hello,
Thank you for correcting patch.
However, I think the following block have to use VirualFree*Ex*().
(yes, this should never happen, maybe there is actually no problem.
but for logical correctness)
+ if (address != UsedShmemSegAddr)
+ {
+ /*
+ * Should never happen - in theory if allocation granularity
causes strange
+ * effects it could, so check just in case.
+ *
+ * Don't use FATAL since we're running in the postmaster.
+ */
+ elog(LOG, reserved shared memory region got incorrect address %p,
expected %p,
+ address, UsedShmemSegAddr);
+ VirtualFree(address, 0, MEM_RELEASE);
VirtualFreeEx(hChild, address, 0, MEM_RELEASE);
+ return false;
+ }
Regards,
--
Tsutomu Yamada
SRA OSS, Inc. Japan
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
On Thu, Jul 23, 2009 at 08:04, Tsutomu Yamadatsut...@sraoss.co.jp wrote: Hello, Thank you for correcting patch. However, I think the following block have to use VirualFree*Ex*(). (yes, this should never happen, maybe there is actually no problem. but for logical correctness) That is definitely correct. I have updated the patch in my tree and will make sure to include that in the eventual commit. FYI, and others, I have received a couple of off-list reports from people testing out the patch, and so far only positive results. -- Magnus Hagander Self: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
On Tue, Jul 21, 2009 at 14:06, Magnus Hagandermag...@hagander.net wrote: On Wed, Jul 15, 2009 at 11:20, Tsutomu Yamadatsut...@sraoss.co.jp wrote: Hello, Alvaro Herrera alvhe...@commandprompt.com 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. This patch looks a lot like one I've had sitting in my tree since before I left for three weeks of vacation, based on the same suggestion on the list. I will check if we have any actual functional differences, and merge yours with mine. The one I had worked fine in my testing. Once that is done, I propose the following: * Apply to HEAD. That will give us buildfarm coverage. * Produce a modified 8.4.0 *and* 8.3.7 binary for this, and ask people to test this. Both people with and without the problem. * Assuming it works for all users, backpatch to 8.2, 8.3 and 8.4. Attached are two updated versions of this patch, one for 8.4 and one for 8.3. They differ only in line numbers. I've merged your patch with mine, which mainly contained of more comments. One functionality check - to make sure the VirtualAllocEx() call returns the same address as our base one. It should always do this, but my patch adds a check t make sure this is true. Dave has built binaries for 8.3.7 and 8.4.0 for this, available at: http://developer.pgadmin.org/~dpage/postgres_exe_virtualalloc-8_3.zip http://developer.pgadmin.org/~dpage/postgres_exe_virtualalloc-8_4.zip We would like as many people as possible to test this both on systems that currently experience the problem and systems that don't, and let us know the status. To test, just replace your current postgres.exe binary with the one in the appropriate ZIP file above. Obviously, take a backup before you do it! These binaries contain just this one patch - the rest of what's been applied to the 8.3 and 8.4 branches for the next minor version is *not* included. -- Magnus Hagander Self: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ *** src/backend/port/win32_shmem.c --- src/backend/port/win32_shmem.c *** *** 18,23 --- 18,24 unsigned long UsedShmemSegID = 0; void *UsedShmemSegAddr = NULL; + static Size UsedShmemSegSize = 0; static void pgwin32_SharedMemoryDelete(int status, Datum shmId); *** *** 229,234 PGSharedMemoryCreate(Size size, bool makePrivate, int port) --- 230,236 /* Save info for possible future use */ UsedShmemSegAddr = memAddress; + UsedShmemSegSize = size; UsedShmemSegID = (unsigned long) hmap2; return hdr; *** *** 253,258 PGSharedMemoryReAttach(void) --- 255,267 Assert(UsedShmemSegAddr != NULL); Assert(IsUnderPostmaster); + /* + * Release memory region reservation that was made by the postmaster + */ + if (VirtualFree(UsedShmemSegAddr, 0, MEM_RELEASE) == 0) + elog(FATAL, 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, *** *** 298,300 pgwin32_SharedMemoryDelete(int status, Datum shmId) --- 307,359 if (!CloseHandle((HANDLE) DatumGetInt32(shmId))) elog(LOG, could not close handle to shared memory: %lu, GetLastError()); } + + /* + * pgwin32_ReserveSharedMemoryRegion(hChild) + * + * Reserve the memory region that will be used for shared memory in a child + * process. It is called before the child process starts, to make sure the + * memory is available. + * + * Once the child starts, DLLs loading in different order or threads getting + * scheduled differently may allocate memory which can conflict with the + * address space we need for our shared memory. By reserving the shared + * memory region before the child starts, and freeing it only
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
On Wed, Jul 15, 2009 at 11:20, Tsutomu Yamadatsut...@sraoss.co.jp wrote: Hello, Alvaro Herrera alvhe...@commandprompt.com 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. This patch looks a lot like one I've had sitting in my tree since before I left for three weeks of vacation, based on the same suggestion on the list. I will check if we have any actual functional differences, and merge yours with mine. The one I had worked fine in my testing. Once that is done, I propose the following: * Apply to HEAD. That will give us buildfarm coverage. * Produce a modified 8.4.0 *and* 8.3.7 binary for this, and ask people to test this. Both people with and without the problem. * Assuming it works for all users, backpatch to 8.2, 8.3 and 8.4. -- Magnus Hagander Self: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
Hello,
Alvaro Herrera alvhe...@commandprompt.com 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 - 1.11
--- src/backend/port/win32_shmem.c 15 Jul 2009 08:56:34 -
***
*** 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 - 1.584
--- src/backend/postmaster/postmaster.c 15 Jul 2009 07:37:09 -
***
*** 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
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
On Tue, Jul 14, 2009 at 6:22 AM, Tsutomu Yamadatsut...@sraoss.co.jp wrote: Hello, This patch using VirtualAlloc()/VirtualFree() to avoid failing in reattach to shared memory. Can this be added to CommitFest ? Patches for CommitFest should be added here: http://commitfest.postgresql.org/action/commitfest_view/open ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
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. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
Alvaro Herrera alvhe...@commandprompt.com writes: Tsutomu Yamada wrote: This patch using VirtualAlloc()/VirtualFree() to avoid failing in reattach to shared memory. Since this fixes a very annoying bug present in older versions, I think this should be backpatched all the way back to 8.2. Agreed, but first we need some evidence that it actually fixes the problem. How can we acquire such evidence? - please use ereport() instead of elog() for error messages This is only appropriate if they're user-facing messages, which probably errors in this area are not ... regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
Tom Lane wrote: Alvaro Herrera alvhe...@commandprompt.com writes: Tsutomu Yamada wrote: This patch using VirtualAlloc()/VirtualFree() to avoid failing in reattach to shared memory. Since this fixes a very annoying bug present in older versions, I think this should be backpatched all the way back to 8.2. Agreed, but first we need some evidence that it actually fixes the problem. How can we acquire such evidence? Send the patch to the people who has reported trouble and see if it seems gone? If somebody is able to build patched Win32 packages I could point a couple of guys in the spanish list to them. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
Alvaro Herrera 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. That doesn't sound like a good idea, at least not before we have more experience of how the patch is working in the field. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
Tom Lane wrote: Alvaro Herrera alvhe...@commandprompt.com writes: Since this fixes a very annoying bug present in older versions, I think this should be backpatched all the way back to 8.2. Agreed, but first we need some evidence that it actually fixes the problem. How can we acquire such evidence? Apply to CVS HEAD and have people test it. I wouldn'ẗ be opposed to back-patching to 8.4 where it would receive more testing in real life. If we're really uneasy about it, provide a switch to turn it off if it causes problems. - please use ereport() instead of elog() for error messages This is only appropriate if they're user-facing messages, which probably errors in this area are not ... Heh, that's what we hope :-). -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
On Tue, Jul 14, 2009 at 10:28 AM, Alvaro Herreraalvhe...@commandprompt.com wrote: Tom Lane wrote: Alvaro Herrera alvhe...@commandprompt.com writes: Tsutomu Yamada wrote: This patch using VirtualAlloc()/VirtualFree() to avoid failing in reattach to shared memory. Since this fixes a very annoying bug present in older versions, I think this should be backpatched all the way back to 8.2. Agreed, but first we need some evidence that it actually fixes the problem. How can we acquire such evidence? Send the patch to the people who has reported trouble and see if it seems gone? If somebody is able to build patched Win32 packages I could point a couple of guys in the spanish list to them. - identify some people with the problem and talk to them for: 1) get a way to reproduce the error (a lot dificult, IIRC we try a few times i fail to fail) or 2) get their support for test - commit it for the first alpha release, or the just talked nigthly stable builds... - let the tests begin :) so, apply it just before the alpha and if it not works remove it just after the alpha... last time i build a win32 binary (not whole package) for windows users to test a patch they disappear very quickly... -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
On Tuesday, July 14, 2009, Alvaro Herrera alvhe...@commandprompt.com wrote: Tom Lane wrote: Alvaro Herrera alvhe...@commandprompt.com writes: Tsutomu Yamada wrote: This patch using VirtualAlloc()/VirtualFree() to avoid failing in reattach to shared memory. Since this fixes a very annoying bug present in older versions, I think this should be backpatched all the way back to 8.2. Agreed, but first we need some evidence that it actually fixes the problem. How can we acquire such evidence? Send the patch to the people who has reported trouble and see if it seems gone? If somebody is able to build patched Win32 packages I could point a couple of guys in the spanish list to them. I built a version which a guy is currently testing. He could reproduce the bug easily, but last i heard, the patch was looking good. Don't have the details here tho. -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] could not reattach to shared memory on Windows
Jaime Casanova wrote: - identify some people with the problem and talk to them for: 1) get a way to reproduce the error (a lot dificult, IIRC we try a few times i fail to fail) or 2) get their support for test For back-patching, we'd be maybe even more interested in getting people who *don't* experience the problem to test it, to make sure it doesn't break installations that work without it. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
