Andres Freund <and...@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 ensure that 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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
