From 61fe442042aa82b09202111fe3656cf464bbd2b9 Mon Sep 17 00:00:00 2001 From: scottmk Date: Mon, 2 Nov 2009 14:41:22 +0000 Subject: [PATCH] Changed the signal handling. There are very few things you can safely do within a signal handler, and shutting down an osrfRouter is not among them. Now the signal handler just sets a switch for the main loop to look at. The select call looks for errno == EINTR and then looks at the switch that the signal handler sets. If the switch is set, we exit the otherwise infinite loop. Then we free the osrfRouter and re-raise the signal. M src/router/osrf_router.h M src/router/osrf_router.c M src/router/osrf_router_main.c git-svn-id: svn://svn.open-ils.org/OpenSRF/trunk@1836 9efc2488-bf62-4759-914b-345cdb29e865 --- src/router/osrf_router.c | 30 ++++++++++++++++++++---- src/router/osrf_router.h | 14 ++++++----- src/router/osrf_router_main.c | 44 ++++++++++++++++++++--------------- 3 files changed, 59 insertions(+), 29 deletions(-) diff --git a/src/router/osrf_router.c b/src/router/osrf_router.c index ace136d..a15c515 100644 --- a/src/router/osrf_router.c +++ b/src/router/osrf_router.c @@ -72,6 +72,18 @@ static int osrfRouterHandleMethodNFound( osrfRouter* router, transport_message* #define ROUTER_REQUEST_STATS_CLASS "opensrf.router.info.stats.class" #define ROUTER_REQUEST_STATS_CLASS_SUMMARY "opensrf.router.info.stats.class.summary" +/** + @brief Stop the otherwise infinite main loop of the router. + @param router Pointer to the osrfRouter to be stopped. + + To be called by a signal handler. +*/ +void router_stop( osrfRouter* router ) +{ + if( router ) + router->stop = 1; +} + /** @brief Allocate and initialize a new osrfRouter. @param domain Domain name of Jabber server @@ -101,6 +113,7 @@ osrfRouter* osrfNewRouter( router->password = strdup(password); router->resource = strdup(resource); router->port = port; + router->stop = 0; router->trustedClients = trustedClients; router->trustedServers = trustedServers; @@ -138,15 +151,25 @@ void osrfRouterRun( osrfRouter* router ) { int routerfd = router->ROUTER_SOCKFD; int selectret = 0; - while(1) { + while( ! router->stop ) { fd_set set; int maxfd = _osrfRouterFillFDSet( router, &set ); int numhandled = 0; if( (selectret = select(maxfd + 1, &set, NULL, NULL, NULL)) < 0 ) { - osrfLogWarning( OSRF_LOG_MARK, "Top level select call failed with errno %d", errno); - continue; + if( EINTR == errno ) { + if( router->stop ) { + osrfLogWarning( OSRF_LOG_MARK, "Top level select call interrupted by signal" ); + break; + } + else + continue; // Irrelevant signal; ignore it + } else { + osrfLogWarning( OSRF_LOG_MARK, + "Top level select call failed with errno %d", errno); + continue; + } } /* see if there is a top level router message */ @@ -157,7 +180,6 @@ void osrfRouterRun( osrfRouter* router ) { osrfRouterHandleIncoming( router ); } - /* now check each of the connected classes and see if they have data to route */ while( numhandled < selectret ) { diff --git a/src/router/osrf_router.h b/src/router/osrf_router.h index fc417c8..d8751f7 100644 --- a/src/router/osrf_router.h +++ b/src/router/osrf_router.h @@ -23,12 +23,13 @@ extern "C" { /* a router maintains a list of server classes */ struct _osrfRouterStruct { - osrfHash* classes; /* our list of server classes */ - char* domain; /* our login domain */ - char* name; - char* resource; - char* password; - int port; + osrfHash* classes; /**< our list of server classes */ + char* domain; /**< Domain name of Jabber server. */ + char* name; /**< Router's username for the Jabber logon. */ + char* resource; /**< Router's resource name for the Jabber logon. */ + char* password; /**< Router's password for the Jabber logon. */ + int port; /**< Jabber's port number. */ + sig_atomic_t stop; /**< To be set by signal handler to interrupt main loop */ osrfStringArray* trustedClients; osrfStringArray* trustedServers; @@ -65,6 +66,7 @@ int osrfRouterConnect( osrfRouter* router ); */ void osrfRouterRun( osrfRouter* router ); +void router_stop( osrfRouter* router ); /** Frees a router diff --git a/src/router/osrf_router_main.c b/src/router/osrf_router_main.c index 0111e8b..5227d0c 100644 --- a/src/router/osrf_router_main.c +++ b/src/router/osrf_router_main.c @@ -28,27 +28,24 @@ */ static osrfRouter* router = NULL; +static sig_atomic_t stop_signal = 0; + +static void setupRouter(jsonObject* configChunk); + /** - @brief Respond to signal by cleaning up and exiting immediately. + @brief Respond to signal by setting a switch that will interrupt the main loop. @param signo The signal number. + + Signal handler. We not only interrupt the main loop but also remember the signal + number so that we can report it later and re-raise it. */ void routerSignalHandler( int signo ) { - osrfLogWarning( OSRF_LOG_MARK, "Received signal [%d], cleaning up...", signo ); - - osrfConfigCleanup(); - osrfRouterFree(router); - osrfLogWarning( OSRF_LOG_MARK, "Cleanup successful. Re-raising signal" ); - router = NULL; - - // Re-raise the signal so that the parent process can detect it. - signal( signo, SIG_DFL ); - raise( signo ); + signal( signo, routerSignalHandler ); + router_stop( router ); + stop_signal = signo; } -static void setupRouter(jsonObject* configChunk); - - /** @brief The top-level function of the router program. @param argc Number of items in command line. @@ -98,12 +95,12 @@ int main( int argc, char* argv[] ) { jsonObject* configChunk = jsonObjectGetIndex(configInfo, i); if( ! jsonObjectGetKey( configChunk, "transport" ) ) { - // In searching the configuration file for a given context, we may have found a spurious - // hit on an unrelated part of the configuration file that happened to use the same XML - // tag. In fact this happens routinely in practice. + // In searching the configuration file for a given context, we may have found a + // spurious hit on an unrelated part of the configuration file that happened to use + // the same XML tag. In fact this happens routinely in practice. - // If we don't see a member for "transport" then this is presumably such a spurious hit, - // so we silently ignore it. + // If we don't see a member for "transport" then this is presumably such a spurious + // hit, so we silently ignore it. // It is also possible that it's the right part of the configuration file but it has a // typo or other such error, making it look spurious. In that case, well, too bad. @@ -115,6 +112,13 @@ int main( int argc, char* argv[] ) { } } + if( stop_signal ) { + // Interrupted by a signal? Re raise so the parent can see it. + osrfLogWarning( OSRF_LOG_MARK, "Interrupted by signal %d; re-raising", + (int) stop_signal ); + raise( stop_signal ); + } + return EXIT_SUCCESS; } @@ -224,6 +228,8 @@ static void setupRouter(jsonObject* configChunk) { osrfRouterFree(router); router = NULL; + + osrfLogInfo( OSRF_LOG_MARK, "Router freed" ); return; } -- 2.43.2