[OPEN-ILS-DEV] PATCH: osrfConfig.C (dropping stderr messages)

[Mike Rylander]

On 6/29/07, Scott McKellar <mck9 at swbell.net> wrote:
>
> --- Mike Rylander <mrylander at gmail.com> wrote:
>
> >> [snip]
> >
> > That is a solid plan.  If you're raising your hand then I'd love to
> > see the patch.  I'd also like to get Bill's input, since this is his
> > code we're beating about, but daemonization needs to be fixed up.
> > */me
> > pokes Bill*
>
> Yeah, I guess I'm raising my hand.  I'll try to put a patch together
> tomorrow evening, unless somebody else gets there first.
>
> Two caveats:
>
> 1. If I were a real systems programmer I wouldn't be digging simple
> stuff out of books.  Please examine closely whatever I send you.

I do and will ... however, you are far from the only systems developer
(real or otherwise) that looks up seeming simple and/or obvious
things.  Looking back at some of my old code, I've written the piece
that closes the terminal FDs and reopens them to /dev/null, but since
I don't write daemons on a regular basis it didn't dawn on me.

>
> 2. I'm not in a position to readily test this kind of change.  Since
> I don't have a library to run, and don't need to set up a functional
> system, my executables tend to be somewhat fragmentary, with things
> stubbed out that I don't need at the moment.  Right now I don't
> have anything that actually calls daemonize(), and it would take me
> a while to set something up.  You should be able to test this change
> better than I can.
>

Actually, there are a couple OpenSRF applications and a jabber server
good enough for testing purposes in the repositiory.  It wouldn't be
much trouble to set up a test environment with just those pieces, and
for the code you're working on (unrelated to jabber or network
communication, other than sockets (as mentioned in Bill's followup)
this would be a great first-pass test.  And, obviously, everything
gets pounded on at dev.gapines.org.

> Upon looking again at daemonize() I realized that the parent process
> calls exit() immediately, so it doesn't need to keep any files open.
> So I think I'll let the parent flush the buffers and call fcloseall(),
> and let the child reopen file descriptors 0, 1, and 2.  I want to
> avoid having both processes flush buffers, because then the same
> data may get flushed twice.  I think.
>
> The remaining issue is that if the fork fails, the present code calls
> perror(), which writes to stderr -- which by that time will have
> been closed.  I'll send the message to the logging system instead.
>

Sounds good.  The only addition I'd like to see would be a check of an
environment variable (OSRF_DEBUG set to 1, perhaps) to disable the
fcloseall/open pair.  Especially now, as we transition from the
current situation, this may be important for debugging.

Bill raised a concern, which I'll adress here, because I'm lazy:

"I'm concerned about the fcloseall() approach.  Currently, we fork the
applications and application worker processes before the top-level
process calls daemonize().  The app processes communicate via pipes to
their worker children (which is something I'd like to re-design, but
I'll save that discussion for later), which, if I'm reading the man
page for fcloseall() correctly, will be closed by that system call. "

Obviously a valid concern, however, I think moving the call to
daemonize to above the forking loop solves this, and ends up giving us
another benefit.  Right now, the controlling process that gets
daemonized is a sibling of the application forkers because daemonize()
is called after the those have been forked off.  Moving the call above
the loop it currently sits below will make the daemon the parent, and
allow things like process group signaling, which is not currently
possible using the "master" daemon process.

-- 
Mike Rylander