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

diff --git a/src/router/osrf_router.c b/src/router/osrf_router.c
index ace136d..a15c515 100644 (file)
--- a/src/router/osrf_router.c
+++ b/src/router/osrf_router.c
@@ -73,6 +73,18 @@ static int osrfRouterHandleMethodNFound( osrfRouter* router, transport_message*
 #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
        @param name Router's username for the Jabber logon.
@@ -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 (file)
--- 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 (file)
--- 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;
 }