Re: retry shm attach for windows (WAS: Re: OK, so culicidae is *still* broken)

On Tue, May 23, 2017 at 8:14 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> So it seems both you and Tom are leaning towards some sort of retry
> mechanism for shm reattach on Windows. I also think that is a viable
> option to negate the impact of ASLR. Attached patch does that. Note
> that, as I have mentioned above I think we need to do it for shm
> reserve operation as well. I think we need to decide how many retries
> are sufficient before bailing out. As of now, I have used 10 to have
> some similarity with PGSharedMemoryCreate(), but we can choose some
> different count as well. One might say that we can have "number of
> retries" as a guc parameter, but I am not sure about it, so not used.

New GUCs can be backpatched if necessary, though this does not seem
necessary. Who is going to set up that anyway if we have a limit high
enough. 10 looks like a sufficient number to me.

> Another point to consider is that do we want the same retry mechanism
> for EXEC_BACKEND builds (function
> PGSharedMemoryReAttach is different on Windows and EXEC_BACKEND
> builds). I think it makes sense to have retry mechanism for
> EXEC_BACKEND builds, so done that way in the patch. Yet another point
> which needs some thought is for reattach operation, before retrying do
> we want to reserve the shm by using VirtualAllocEx?

- elog(FATAL, "could not reattach to shared memory (key=%p,
addr=%p): error code %lu",
+ {
+ elog(LOG, "could not reattach to shared memory (key=%p,
addr=%p): error code %lu",
UsedShmemSegID, UsedShmemSegAddr, GetLastError());
+ return false;
+ }
This should be a WARNING, with the attempt number reported as well?

-void
-PGSharedMemoryReAttach(void)
+bool
+PGSharedMemoryReAttach(int retry_count)
I think that the loop logic should be kept within
PGSharedMemoryReAttach, this makes the code of postmaster.c cleaner,
and it seems to me that each step of PGSharedMemoryReAttach() should
be retried in order. Do we need also to worry about SysV? I agree with
you that having consistency is better, but I don't recall seeing
failures or complains related to cygwin for ASLR.

I think that you are forgetting PGSharedMemoryCreate in the retry process.
--
Michael