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

[Mike Rylander]

On Jan 11, 2008 12:20 AM, Scott McKellar <mck9 at swbell.net> wrote:
> This patch plugs some memory leaks, rearranges the signal handling,
> and tidies up various things.

Applied in its entirety, including re-raising of the signal.

>
> 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.



-- 
Mike Rylander
 | VP, Research and Design
 | Equinox Software, Inc. / The Evergreen Experts
 | phone:  1-877-OPEN-ILS (673-6457)
 | email:  miker at esilibrary.com
 | web:  http://www.esilibrary.com