[PATCH] ctdb-common: Set close-on-exec when creating PID file (bug 12898)

[Volker Lendecke]

On Thu, Jul 13, 2017 at 08:37:57PM +1000, Martin Schwenke wrote:
> * API
> 
>   We really just want a file pathname instead of directory and basename.
>   The main reason for this is that initscripts and systemd unit files
>   (and. consequently, ctdbd_wrapper) need to know the full pathname, so
>   it doesn't make sense to split it and re-join it.

Fine with that. All callers of lib/util/pidfile.c use
lp_pid_directory() or the source4 equivalent. That is easily
wrappable.

> * Coverity hit
> 
>   lib/util/pidfile.c currently leaks the file descriptor and Coverity
>   notices.  Amitay's implementation stores it in a context.  When the
>   context is freed then the destructor does unlink/unlock/close.

To be honest, I see this as a cosmetic issue. The pidfile is kept open
by the parent process deliberately. It's a bit like the
autofree_context discussion: Depending on proper rundown by a talloc
destructor is important for more dynamic things. We should not depend
on this for main process exit. Other opinions? I'd call this a
"deliberate" in Coverity.

> 
> * Fear  ;-)
> 
>   lib/util/pidfile.c has this at the top:
> 
>   /* this code is broken - there is a race condition with the unlink
>   (tridge) */

Not sure this still is true. If the ctdb version does this better, I'm
more than happy to port the fix.

Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de