Andres Freund <andres@anarazel.de> writes:
> On 2015-10-12 21:38:12 +0900, Michael Paquier wrote:
>> Actually, doesn't this apply as well to the archiver and the pgstat
>> collector?
> As mentioned above? The difference is that the archiver et al get killed
> by postmaster during a PANIC restart thus don't present the problem
> discussed here.
I thought your objection to the original patch was exactly that we should
not treat syslogger as a special case for this purpose.
> Well, in those cases we won't have attached to shared memory, so I'm not
> convinced that this is the right solution.
No, you're missing the point. In Windows builds, child processes inherit
a "handle" reference to the shared memory mapping, whether or not they
make any use of the handle to re-attach to that shared memory. The point
here is that we need to close that handle if we're not going to use it.
I think the right thing is something close to Michael's proposed patch,
though not duplicating and reversing the previous if-test like that.
In other words, something like this in SubPostmasterMain:
/* * If appropriate, physically re-attach to shared memory segment. We want * to do this before going any further to
ensurethat we can attach at the * same address the postmaster used.
+ * If we're not re-attaching, close the inherited handle to avoid leaks. */if (strcmp(argv[1], "--forkbackend") ==
0|| strcmp(argv[1], "--forkavlauncher") == 0 || strcmp(argv[1], "--forkavworker") == 0 || strcmp(argv[1],
"--forkboot")== 0 || strncmp(argv[1], "--forkbgworker=", 15) == 0) PGSharedMemoryReAttach();
+#ifdef WIN32
+ else
+ close the handle;
+#endif
regards, tom lane