On Fri, Jan 31, 2014 at 6:15 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> This looks good to me in principle. A couple minor beefs:
>
> * The addition to CleanupProcSignalState could use a comment,
> similar to the one you added in ProcKill.
OK.
> * I think the code in ProcKill and AuxiliaryProcKill might be more
> readable if the new local variable was named "myproc" (lower case).
grep indicates that naming is less common that what I picked, so I
chose to stick with what I picked.
>> and we can easily add a NULL guard to the SetLatch() call in
>> procsignal_sigusr1_handler, which the attached patch also does.
>
> Um ... no such change actually visible in patch, but it's clearly
> necessary.
Fixed.
>> This might not be a complete fix to every problem of this type that
>> exists anywhere in our code, but I think it's enough to make the world
>> safe for procsignal_sigusr1_handler.
>
> Yeah; at the least this should cut down on the buildfarm noise we
> are seeing ATM.
>
>> Assuming nobody objects too much to this basic approach, should I
>> back-patch the parts of this that apply pre-9.4?
>
> Yes, I think so. We have seen some reports of irreproducible crashes
> at process exit, and maybe this explains them.
OK, done.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
