From: | Peter Geoghegan <peter(at)2ndquadrant(dot)com> |
---|---|
To: | Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> |
Cc: | PG Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Latch implementation that wakes on postmaster death on both win32 and Unix |
Date: | 2011-06-16 12:07:17 |
Message-ID: | BANLkTimi_CgmEV-XQviTAtPb=v16=XkGAQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
I had another quick look-over this patch, and realised that I made a
minor mistake:
+void
+ReleasePostmasterDeathWatchHandle(void)
+{
+ /* MyProcPid won't have been set yet */
+ Assert(PostmasterPid != getpid());
+ /* Please don't ask twice */
+ Assert(postmaster_alive_fds[POSTMASTER_FD_OWN] != -1);
+ /* Release parent's ownership fd - only postmaster should hold it */
+ if (close(postmaster_alive_fds[ POSTMASTER_FD_OWN]))
+ {
+ ereport(FATAL,
+ (errcode_for_socket_access(),
+ errmsg("Failed to close file descriptor associated with
Postmaster death in child process %d", MyProcPid)));
+ }
+ postmaster_alive_fds[POSTMASTER_FD_OWN] = -1;
+}
+
MyProcPid is used in this errmsg, and as noted in the first comment,
it isn't expected to be initialised when
ReleasePostmasterDeathWatchHandle() is called. Therefore, MyProcPid
should be replaced with a call to getpid(), just as it is for
Assert(PostmasterPid != getpid()).
I suppose that you could take the view that MyProcPid ought to be
initialised before the function is called, but I thought this was the
least worst way. Better to do it this way than to touch all the
different ways in which MyProcPid might be initialised, I suspect.
--
Peter Geoghegan http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services
From | Date | Subject | |
---|---|---|---|
Next Message | Heikki Linnakangas | 2011-06-16 12:15:44 | Re: Latch implementation that wakes on postmaster death on both win32 and Unix |
Previous Message | Florian Pflug | 2011-06-16 11:34:51 | Re: proposal: a validator for configuration files |