[OPEN-ILS-DEV] PATCH: osrf_router_main.c (miscellaneous)

[Scott McKellar]

This patch plugs some memory leaks, rearranges the signal handling,
and tidies up various things.

1. I renamed __osrfRouter simply to router, and made it static.  We 
had one global variable and one auto variable pointing to the same 
object, causing some needless juggling.  Now we have just one
pointer.

2. I removed the leading underscores from __setupRouter().

3. I renamed the parameter to routerSignalHandler() from "signal"
to "signo", since "signal" is a reserved identifier.  I also added
some code to re-raise the signal caught.

4. In main() I moved two calls to free() so that they are reachable.

5. In main() I return either EXIT_SUCCESS or EXIT_FAILURE, which are
portable.  Otherwise we could find ourselves returning -1, which is
not portable.

6. In setupRouter() I arranged to free resource, and to free 
tservers and tclients in the case of an early return.  I also free
router in the unlikely event that osrfRouterRun returns.

7. I reworded an error message to something that I think is more
clear.

----------

I don't know what happens if we return -1 from main().  Traditional
UNIX (and I presume Linux) recognizes return codes from 0 to 255.
Probably -1 would get converted to 255.  In any case the result
depends on the C implementation.  I'd rather use the standard macros
EXIT_SUCCESS and EXIT_FAILURE. 

----------

In the original, the signal handler would destroy the configuration
and the osrfRouter and then return.  The rest of the program would
then resume whatever it was doing from the point of interruption --
except that the configuration and the osrfRouter had been destroyed
out from under it.

This approach presumably achieves the intended objective of 
terminating the program, but not in the best way possible.

Another approach is for the signal handler to call exit() or abort().
I chose instead to terminate the program by re-raising the signal.
That way the parent process has a chance to detect that the program
was terminated by a signal rather than by a normal return.

-----------

While this approach should provide for a more graceful termination,
I'm still not very happy with it because the signal handler calls
functions that aren't re-entrant.  Imagine the following scenario:

1. We receive a SIGTERM.  The signal handler catches it and starts
to do its thing.

2. Midway through the destruction of the osrfRouter we receive
a SIGHUP.  The signal handler catches it and starts over from the
top.

3. We wind up trying to free something that we already freed while
responding to the original SIGTERM.  Oops.

A proper signal handler can safely do only a very limited number 
of things.  It can call reentrant functions; it can call exit(), 
abort(), or longjump(); or it can update  the value of one or more
variables of type sig_atomic_t.  Anything else risks corruption by
a second signal received during the processing of the first signal.

In this case a more robust approach would be to install the signal
handler in osrf_router.c.  The signal handler itself would simply
set a flag of type sig_atomic_t and then set the signal handling
to SIG_IGN.  Each iteration of the main loop (which is currently an
infinite loop) would check the flag.  If the flag indicates that a 
signal has been caught, we would exit the loop or otherwise 
terminate.  Ideally we wouldn't re-raise an exception until we had
freed all resources and closed all files.  I haven't worked out all
the details.

Scott McKellar
http://home.swbell.net/mck9/ct/

Developer's Certificate of Origin 1.1 By making a contribution to
this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license indicated
in the file; or

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source license
and I have the right under that license to submit that work with
modifications, whether created in whole or in part by me, under the
same open source license (unless I am permitted to submit under a
different license), as indicated in the file; or

(c) The contribution was provided directly to me by some other person
who certified (a), (b) or (c) and I have not modified it; and

(d) In the case of each of (a), (b), or (c), I understand and agree
that this project and the contribution are public and that a record
of the contribution (including all personal information I submit
with it, including my sign-off) is maintained indefinitely and may
be redistributed consistent with this project or the open source
license indicated in the file.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: osrf_router_main_c_1.patch
Type: text/x-patch
Size: 3983 bytes
Desc: 38467175-osrf_router_main_c_1.patch
Url : http://list.georgialibraries.org/pipermail/open-ils-dev/attachments/20080110/bc5418d9/osrf_router_main_c_1.bin