Re: [PATCH] "could not reattach to shared memory" on Windows - Mailing list pgsql-hackers

From Magnus Hagander
Subject Re: [PATCH] "could not reattach to shared memory" on Windows
Date
Msg-id 9837222c0907210506t53cf5008s2c8cc77a80eaf6@mail.gmail.com
Whole thread Raw
In response to Re: [PATCH] "could not reattach to shared memory" on Windows  (Tsutomu Yamada <tsutomu@sraoss.co.jp>)
Responses Re: [PATCH] "could not reattach to shared memory" on Windows
List pgsql-hackers
On Wed, Jul 15, 2009 at 11:20, Tsutomu Yamada<tsutomu@sraoss.co.jp> wrote:
> Hello,
>
> Alvaro Herrera <alvherre@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 HaganderSelf: http://www.hagander.net/Work: http://www.redpill-linpro.com/


pgsql-hackers by date:

Previous
From: Dimitri Fontaine
Date:
Subject: Re: Sampling profiler updated
Next
From: pg@thetdh.com
Date:
Subject: Re: [PATCH v4] Avoid manual shift-and-test logic in AllocSetFreeIndex