1. In the parent router process: wait for all of the immediate
[OpenSRF.git] / src / router / osrf_router.c
index bbc9446..ede2ee3 100644 (file)
@@ -1,4 +1,14 @@
+#include <sys/select.h>
+#include <signal.h>
+#include "opensrf/utils.h"
+#include "opensrf/log.h"
+#include "opensrf/osrf_list.h"
+#include "opensrf/string_array.h"
+#include "opensrf/osrf_hash.h"
 #include "osrf_router.h"
+#include "opensrf/transport_client.h"
+#include "opensrf/transport_message.h"
+#include "opensrf/osrf_message.h"
 
 /**
        @file osrf_router.c
        each server class.  The Jabber IDs for these sessions are distinguished by the use of
        the class names as Jabber resource names.
 
-       For each server class there may be multiple server nodes.
+       For each server class there may be multiple server nodes.  Each node corresponds to a
+       listener process for a service.
 */
 
-/* a router maintains a list of server classes */
-
 /**
        @brief Collection of server classes, with connection parameters for Jabber.
  */
@@ -25,18 +34,20 @@ struct osrfRouterStruct {
                osrfRouterClass.
        */
        osrfHash* classes;
-       osrfHashIterator* class_itr;  /**< For traversing the list of classes */
+       osrfHashIterator* class_itr;  /**< For traversing the list of 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 */
+       volatile sig_atomic_t stop; /**< To be set by signal handler to interrupt main loop. */
 
        /** Array of client domains that we allow to send requests through us. */
        osrfStringArray* trustedClients;
        /** Array of server domains that we allow to register, etc. with us. */
        osrfStringArray* trustedServers;
+       /** List of osrfMessages to be returned from osrfMessageDeserialize() */
+       osrfList* message_list;
 
        transport_client* connection;
 };
@@ -71,11 +82,11 @@ typedef struct _osrfRouterNodeStruct osrfRouterNode;
 
 static osrfRouterClass* osrfRouterAddClass( osrfRouter* router, const char* classname );
 static void osrfRouterClassAddNode( osrfRouterClass* rclass, const char* remoteId );
-static void osrfRouterHandleCommand( osrfRouter* router, transport_message* msg );
-static int osrfRouterClassHandleMessage( osrfRouter* router,
-               osrfRouterClass* rclass, transport_message* msg );
+static void osrfRouterHandleCommand( osrfRouter* router, const transport_message* msg );
+static void osrfRouterClassHandleMessage( osrfRouter* router,
+               osrfRouterClass* rclass, const transport_message* msg );
 static void osrfRouterRemoveClass( osrfRouter* router, const char* classname );
-static int osrfRouterClassRemoveNode( osrfRouter* router, const char* classname,
+static void osrfRouterClassRemoveNode( osrfRouter* router, const char* classname,
                const char* remoteId );
 static void osrfRouterClassFree( char* classname, void* rclass );
 static void osrfRouterNodeFree( char* remoteId, void* node );
@@ -84,19 +95,19 @@ static osrfRouterNode* osrfRouterClassFindNode( osrfRouterClass* rclass,
                const char* remoteId );
 static int _osrfRouterFillFDSet( osrfRouter* router, fd_set* set );
 static void osrfRouterHandleIncoming( osrfRouter* router );
-static int osrfRouterClassHandleIncoming( osrfRouter* router,
+static void osrfRouterClassHandleIncoming( osrfRouter* router,
                const char* classname,  osrfRouterClass* class );
 static transport_message* osrfRouterClassHandleBounce( osrfRouter* router,
-               const char* classname, osrfRouterClass* rclass, transport_message* msg );
-static void osrfRouterHandleAppRequest( osrfRouter* router, transport_message* msg );
-static int osrfRouterRespondConnect( osrfRouter* router, transport_message* msg,
-               osrfMessage* omsg );
-static int osrfRouterProcessAppRequest( osrfRouter* router, transport_message* msg,
-               osrfMessage* omsg );
-static int osrfRouterHandleAppResponse( osrfRouter* router,
-               transport_message* msg, osrfMessage* omsg, const jsonObject* response );
-static int osrfRouterHandleMethodNFound( osrfRouter* router, transport_message* msg,
-               osrfMessage* omsg );
+               const char* classname, osrfRouterClass* rclass, const transport_message* msg );
+static void osrfRouterHandleAppRequest( osrfRouter* router, const transport_message* msg );
+static void osrfRouterRespondConnect( osrfRouter* router, const transport_message* msg,
+               const osrfMessage* omsg );
+static void osrfRouterProcessAppRequest( osrfRouter* router, const transport_message* msg,
+               const osrfMessage* omsg );
+static void osrfRouterSendAppResponse( osrfRouter* router, const transport_message* msg,
+               const osrfMessage* omsg, const jsonObject* response );
+static void osrfRouterHandleMethodNFound( osrfRouter* router,
+               const transport_message* msg, const osrfMessage* omsg );
 
 #define ROUTER_REGISTER "register"
 #define ROUTER_UNREGISTER "unregister"
@@ -108,10 +119,11 @@ 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.
+       @brief Stop the otherwise endless main loop of the router.
        @param router Pointer to the osrfRouter to be stopped.
 
-       To be called by a signal handler.
+       To be called by a signal handler.  We don't stop the loop immediately; we just set
+       a switch that the main loop checks on each iteration.
 */
 void router_stop( osrfRouter* router )
 {
@@ -121,14 +133,14 @@ void router_stop( osrfRouter* router )
 
 /**
        @brief Allocate and initialize a new osrfRouter.
-       @param domain Domain name of Jabber server
+       @param domain Domain name of Jabber server.
        @param name Router's username for the Jabber logon.
        @param resource Router's resource name for the Jabber logon.
        @param password Router's password for the Jabber logon.
        @param port Jabber's port number.
        @param trustedClients Array of client domains that we allow to send requests through us.
        @param trustedServers Array of server domains that we allow to register, etc. with us.
-       @return Pointer to the newly allocated osrfRouter.
+       @return Pointer to the newly allocated osrfRouter, or NULL upon error.
 
        Don't connect to Jabber yet.  We'll do that later, upon a call to osrfRouterConnect().
 
@@ -157,6 +169,7 @@ osrfRouter* osrfNewRouter(
        router->classes = osrfNewHash();
        osrfHashSetCallback(router->classes, &osrfRouterClassFree);
        router->class_itr = osrfNewHashIterator( router->classes );
+       router->message_list = NULL;   // We'll allocate one later
 
        // Prepare to connect to Jabber, as a non-component, over TCP (not UNIX domain).
        router->connection = client_init( domain, port, NULL, 0 );
@@ -166,7 +179,7 @@ osrfRouter* osrfNewRouter(
 
 /**
        @brief Connect to Jabber.
-       @param router Pointer to the osrfRouter to connect to Jabber
+       @param router Pointer to the osrfRouter to connect to Jabber.
        @return 0 if successful, or -1 on error.
 
        Allow up to 10 seconds for the logon to succeed.
@@ -184,10 +197,9 @@ int osrfRouterConnect( osrfRouter* router ) {
 /**
        @brief Enter endless loop to receive and respond to input.
        @param router Pointer to the osrfRouter that's looping.
-       @return 
 
        On each iteration: wait for incoming messages to arrive on any of our sockets -- i.e.
-       either the top level socket belong to the router or any of the lower level sockets
+       either the top level socket belonging to the router or any of the lower level sockets
        belonging to the classes.  React to the incoming activity as needed.
 
        We don't exit the loop until we receive a signal to stop, or until we encounter an error.
@@ -203,8 +215,8 @@ void osrfRouterRun( osrfRouter* router ) {
 
                fd_set set;
                int maxfd = _osrfRouterFillFDSet( router, &set );
-               int numhandled = 0;
 
+               // Wait indefinitely for an incoming message
                if( (selectret = select(maxfd + 1, &set, NULL, NULL, NULL)) < 0 ) {
                        if( EINTR == errno ) {
                                if( router->stop ) {
@@ -221,36 +233,30 @@ void osrfRouterRun( osrfRouter* router ) {
                }
 
                /* see if there is a top level router message */
-
                if( FD_ISSET(routerfd, &set) ) {
                        osrfLogDebug( OSRF_LOG_MARK, "Top router socket is active: %d", routerfd );
-                       numhandled++;
                        osrfRouterHandleIncoming( router );
                }
 
-               /* now check each of the connected classes and see if they have data to route */
-               while( numhandled < selectret ) {
-
-                       osrfRouterClass* class;
-                       osrfHashIterator* itr = router->class_itr;  // remove a layer of indirection
-                       osrfHashIteratorReset( itr );
+               /* Check each of the connected classes and see if they have data to route */
+               osrfRouterClass* class;
+               osrfHashIterator* itr = router->class_itr;  // remove a layer of indirection
+               osrfHashIteratorReset( itr );
 
-                       while( (class = osrfHashIteratorNext(itr)) ) {
+               while( (class = osrfHashIteratorNext(itr)) ) {   // for each class
 
-                               const char* classname = osrfHashIteratorKey(itr);
+                       const char* classname = osrfHashIteratorKey(itr);
 
-                               if( classname ) {
+                       if( classname ) {
 
-                                       osrfLogDebug( OSRF_LOG_MARK, "Checking %s for activity...", classname );
+                               osrfLogDebug( OSRF_LOG_MARK, "Checking %s for activity...", classname );
 
-                                       int sockfd = client_sock_fd( class->connection );
-                                       if(FD_ISSET( sockfd, &set )) {
-                                               osrfLogDebug( OSRF_LOG_MARK, "Socket is active: %d", sockfd );
-                                               numhandled++;
-                                               osrfRouterClassHandleIncoming( router, classname, class );
-                                       }
+                               int sockfd = client_sock_fd( class->connection );
+                               if(FD_ISSET( sockfd, &set )) {
+                                       osrfLogDebug( OSRF_LOG_MARK, "Socket is active: %d", sockfd );
+                                       osrfRouterClassHandleIncoming( router, classname, class );
                                }
-                       } // end while
+                       }
                } // end while
        } // end while
 }
@@ -278,7 +284,7 @@ static void osrfRouterHandleIncoming( osrfRouter* router ) {
                        osrfLogDebug(OSRF_LOG_MARK,
                                "osrfRouterHandleIncoming(): investigating message from %s", msg->sender);
 
-                       /* if the sender is not on a trusted domain, drop the message */
+                       /* if the server is not on a trusted domain, drop the message */
                        int len = strlen(msg->sender) + 1;
                        char domain[len];
                        jid_get_domain( msg->sender, domain, len - 1 );
@@ -302,22 +308,25 @@ static void osrfRouterHandleIncoming( osrfRouter* router ) {
 }
 
 /**
-       @brief Handle incoming requests to a router class.
+       @brief Handle all available incoming messages for a router class.
        @param router Pointer to the osrfRouter.
        @param classname Class name.
-       @param class Pointer to an osrfRouterClass.
+       @param class Pointer to the osrfRouterClass.
 
-       Make sure sender is a trusted client.
+       For each message: if the sender is on a trusted domain, process the message.
 */
-static int osrfRouterClassHandleIncoming( osrfRouter* router, const char* classname,
+static void osrfRouterClassHandleIncoming( osrfRouter* router, const char* classname,
                osrfRouterClass* class ) {
-       if(!(router && class)) return -1;
+       if(!(router && class)) return;
 
        transport_message* msg;
        osrfLogDebug( OSRF_LOG_MARK, "osrfRouterClassHandleIncoming()");
 
+       // For each incoming message for this class:
        while( (msg = client_recv( class->connection, 0 )) ) {
 
+               // Save the transaction id so that we can incorporate it
+               // into any relevant messages
                osrfLogSetXid(msg->osrf_xid);
 
                if( msg->sender ) {
@@ -328,23 +337,27 @@ static int osrfRouterClassHandleIncoming( osrfRouter* router, const char* classn
                        /* if the client is not from a trusted domain, drop the message */
                        int len = strlen(msg->sender) + 1;
                        char domain[len];
-                       memset(domain, 0, sizeof(domain));
                        jid_get_domain( msg->sender, domain, len - 1 );
 
-                       if(osrfStringArrayContains( router->trustedClients, domain)) {
+                       if(osrfStringArrayContains( router->trustedClients, domain )) {
 
-                               transport_message* bouncedMessage = NULL;
                                if( msg->is_error )  {
 
+                                       // A previous message bounced.  Try to send a clone of it to a
+                                       // different node of the same class.
+                                       transport_message* bouncedMessage = osrfRouterClassHandleBounce(
+                                                       router, classname, class, msg );
                                        /* handle bounced message */
-                                       if( !(bouncedMessage = osrfRouterClassHandleBounce( router, classname,
-                                                       class, msg )) )
-                                               return -1; /* we have no one to send the requested message to */
-
-                                       message_free( msg );
-                                       msg = bouncedMessage;
-                               }
-                               osrfRouterClassHandleMessage( router, class, msg );
+                                       if( !bouncedMessage ) {
+                                               /* we have no one to send the requested message to */
+                                               message_free( msg );
+                                               osrfLogClearXid();
+                                               continue;
+                                       }
+                                       osrfRouterClassHandleMessage( router, class, bouncedMessage );
+                                       message_free( bouncedMessage );
+                               } else
+                                       osrfRouterClassHandleMessage( router, class, msg );
 
                        } else {
                                osrfLogWarning( OSRF_LOG_MARK, 
@@ -352,11 +365,9 @@ static int osrfRouterClassHandleIncoming( osrfRouter* router, const char* classn
                        }
                }
 
-               osrfLogClearXid();
                message_free( msg );
+               osrfLogClearXid();  // We're done with this transaction id
        }
-
-       return 0;
 }
 
 /**
@@ -366,9 +377,10 @@ static int osrfRouterClassHandleIncoming( osrfRouter* router, const char* classn
 
        Currently supported commands:
        - "register" -- Add a server class and/or a server node to our lists.
-       - "unregister" -- Remove a server class (and any associated nodes) from our list.
+       - "unregister" -- Remove a node from a class, and the class as well if no nodes are
+       left for it.
 */
-static void osrfRouterHandleCommand( osrfRouter* router, transport_message* msg ) {
+static void osrfRouterHandleCommand( osrfRouter* router, const transport_message* msg ) {
        if(!(router && msg && msg->router_class)) return;
 
        if( !strcmp( msg->router_command, ROUTER_REGISTER ) ) {
@@ -395,7 +407,7 @@ static void osrfRouterHandleCommand( osrfRouter* router, transport_message* msg
 
 
 /**
-       @brief Adds an osrfRouterClass to a router, and open a connection for it.
+       @brief Add an osrfRouterClass to a router, and open a connection for it.
        @param router Pointer to the osrfRouter.
        @param classname The name of the class this node handles.
        @return A pointer to the new osrfRouterClass, or NULL upon error.
@@ -432,7 +444,7 @@ static osrfRouterClass* osrfRouterAddClass( osrfRouter* router, const char* clas
 
 /**
        @brief Add a new server node to an osrfRouterClass.
-       @param rclass Pointer to the osrfRouterClass to add the node to.
+       @param rclass Pointer to the osrfRouterClass to which we are to add the node.
        @param remoteId The remote login of the osrfRouterNode.
 */
 static void osrfRouterClassAddNode( osrfRouterClass* rclass, const char* remoteId ) {
@@ -448,25 +460,35 @@ static void osrfRouterClassAddNode( osrfRouterClass* rclass, const char* remoteI
        osrfHashSet( rclass->nodes, node, remoteId );
 }
 
-/* copy off the lastMessage, remove the offending node, send error if it's tht last node
-       ? return NULL if it's the last node ?
-*/
-
-/* handles case where router node is not longer reachable.  copies over the
-       data from the last sent message and returns a newly crafted suitable for treating
-       as a newly inconing message.  Removes the dead node and If there are no more
-       nodes to send the new message to, returns NULL.
+/**
+       @brief Handle an input message representing a Jabber error stanza.
+       @param router Pointer to the current osrfRouter.
+       @param classname Name of the class to which the error stanza was sent.
+       @param rclass Pointer to the osrfRouterClass to which the error stanza was sent.
+       @param msg Pointer to the transport_message representing the error stanza.
+       @return Pointer to a newly allocated transport_message; or NULL (see remarks).
+
+       The presumption is that the relevant node is dead.  If another node is available for
+       the same class, then remove the dead one, create a clone of the message to be sent
+       elsewhere, and return a pointer to it.  If there is no other node for the same class,
+       send a cancel message back to the sender, remove both the node and the class it belongs
+       to, and return NULL.  If we can't even do that because the entire class is dead, log
+       a message to that effect and return NULL.
 */
 static transport_message* osrfRouterClassHandleBounce( osrfRouter* router,
-               const char* classname, osrfRouterClass* rclass, transport_message* msg ) {
+               const char* classname, osrfRouterClass* rclass, const transport_message* msg ) {
 
        osrfLogDebug( OSRF_LOG_MARK, "osrfRouterClassHandleBounce()");
 
        osrfLogInfo( OSRF_LOG_MARK, "Received network layer error message from %s", msg->sender );
        osrfRouterNode* node = osrfRouterClassFindNode( rclass, msg->sender );
-       transport_message* lastSent = NULL;
+       if( ! node ) {
+               osrfLogInfo( OSRF_LOG_MARK,
+                       "network error occurred after we removed the class.. ignoring");
+               return NULL;
+       }
 
-       if( node && osrfHashGetCount(rclass->nodes) == 1 ) { /* the last node is dead */
+       if( osrfHashGetCount(rclass->nodes) == 1 ) { /* the last node is dead */
 
                if( node->lastMessage ) {
                        osrfLogWarning( OSRF_LOG_MARK,
@@ -484,54 +506,60 @@ static transport_message* osrfRouterClassHandleBounce( osrfRouter* router,
                        message_free( error );
                }
 
+               /* remove the dead node */
+               osrfRouterClassRemoveNode( router, classname, msg->sender);
                return NULL;
 
        } else {
 
-               if( node ) {
-                       if( node->lastMessage ) {
-                               osrfLogDebug( OSRF_LOG_MARK, "Cloning lastMessage so next node can send it");
-                               lastSent = message_init( node->lastMessage->body,
-                                       node->lastMessage->subject, node->lastMessage->thread, "", node->lastMessage->router_from );
-                               message_set_router_info( lastSent, node->lastMessage->router_from, NULL, NULL, NULL, 0 );
-                       message_set_osrf_xid( lastSent, node->lastMessage->osrf_xid );
-                       }
-               } else {
+               transport_message* lastSent = NULL;
 
-                       osrfLogInfo(OSRF_LOG_MARK, "network error occurred after we removed the class.. ignoring");
-                       return NULL;
+               if( node->lastMessage ) {
+                       osrfLogDebug( OSRF_LOG_MARK, "Cloning lastMessage so next node can send it");
+                       lastSent = message_init( node->lastMessage->body,
+                               node->lastMessage->subject, node->lastMessage->thread, "",
+                               node->lastMessage->router_from );
+                       message_set_router_info( lastSent, node->lastMessage->router_from,
+                               NULL, NULL, NULL, 0 );
+                       message_set_osrf_xid( lastSent, node->lastMessage->osrf_xid );
                }
-       }
 
-       /* remove the dead node */
-       osrfRouterClassRemoveNode( router, classname, msg->sender);
-       return lastSent;
+               /* remove the dead node */
+               osrfRouterClassRemoveNode( router, classname, msg->sender);
+               return lastSent;
+       }
 }
 
 
-/*
-       Handles class level requests
-       If we get a regular message, we send it to the next node in the list of nodes
-       if we get an error, it's a bounce back from a previous attempt.  We take the
-       body and thread from the last sent on the node that had the bounced message
-       and propogate them on to the new message being sent
-       @return 0 on success
+/**
+       @brief Forward a class-level message to a listener for the corresponding service.
+       @param router Pointer to the current osrfRouter.
+       @param rclass Pointer to the class to which the message is directed.
+       @param msg Pointer to the message to be forwarded.
+
+       Pick a node for the specified class, and forward the message to it.
+
+       We use an iterator, stored with the class, to maintain a position in the class's list
+       of nodes.  Advance the iterator to pick the next node, and if we reach the end, go
+       back to the beginning of the list.
 */
-static int osrfRouterClassHandleMessage(
-               osrfRouter* router, osrfRouterClass* rclass, transport_message* msg ) {
-       if(!(router && rclass && msg)) return -1;
+static void osrfRouterClassHandleMessage(
+               osrfRouter* router, osrfRouterClass* rclass, const transport_message* msg ) {
+       if(!(router && rclass && msg)) return;
 
        osrfLogDebug( OSRF_LOG_MARK, "osrfRouterClassHandleMessage()");
 
+       // Pick a node, in a round-robin.
        osrfRouterNode* node = osrfHashIteratorNext( rclass->itr );
-       if(!node) {
+       if(!node) {   // wrap around to the beginning of the list
                osrfHashIteratorReset(rclass->itr);
                node = osrfHashIteratorNext( rclass->itr );
        }
 
-       if(node) {
+       if(node) {  // should always be true -- no class without a node
 
-               transport_message* new_msg= message_init( msg->body,
+               // Build a transport message
+               transport_message* new_msg = message_init( msg->body,
                                msg->subject, msg->thread, node->remoteId, msg->sender );
                message_set_router_info( new_msg, msg->sender, NULL, NULL, NULL, 0 );
                message_set_osrf_xid( new_msg, msg->osrf_xid );
@@ -539,9 +567,11 @@ static int osrfRouterClassHandleMessage(
                osrfLogInfo( OSRF_LOG_MARK,  "Routing message:\nfrom: [%s]\nto: [%s]",
                                new_msg->router_from, new_msg->recipient );
 
+               // Save it for possible future reference
                message_free( node->lastMessage );
                node->lastMessage = new_msg;
 
+               // Send it
                if ( client_send_message( rclass->connection, new_msg ) == 0 )
                        node->count++;
 
@@ -550,10 +580,8 @@ static int osrfRouterClassHandleMessage(
                        osrfLogWarning( OSRF_LOG_MARK, "Error sending message from %s to %s\n%s",
                                        new_msg->sender, new_msg->recipient, new_msg->msg_xml );
                }
-
+               // We don't free new_msg here because we saved it as node->lastMessage.
        }
-
-       return 0;
 }
 
 
@@ -562,8 +590,8 @@ static int osrfRouterClassHandleMessage(
        @param router Pointer to the osrfRouter.
        @param classname The name of the class to be removed.
 
-       A callback function, installed in the osrfHash, frees the osrfRouterClass and any
-       associated nodes.
+       Delete an osrfRouterClass from the router's list of classes.  Indirectly (via a callback
+       function installed in the osrfHash), free the osrfRouterClass and any associated nodes.
 */
 static void osrfRouterRemoveClass( osrfRouter* router, const char* classname ) {
        if( router && router->classes && classname ) {
@@ -573,46 +601,41 @@ static void osrfRouterRemoveClass( osrfRouter* router, const char* classname ) {
 }
 
 
-/*
-       Removes the given node from the class.  Also, if this is that last node in the set,
-       removes the class from the router
-       @return 0 on successful removal with no class removal
-       @return 1 on successful remove with class removal
-       @return -1 error on removal
+/**
+       @brief Remove a node from a class.  If the class thereby becomes empty, remove it as well.
+       @param router Pointer to the current osrfRouter.
+       @param classname Class name.
+       @param remoteId Identifier for the node to be removed.
 */
-static int osrfRouterClassRemoveNode(
+static void osrfRouterClassRemoveNode(
                osrfRouter* router, const char* classname, const char* remoteId ) {
 
-       if(!(router && router->classes && classname && remoteId)) return 0;
+       if(!(router && router->classes && classname && remoteId))  // sanity check
+               return;
 
        osrfLogInfo( OSRF_LOG_MARK, "Removing router node %s", remoteId );
 
        osrfRouterClass* class = osrfRouterFindClass( router, classname );
-
        if( class ) {
-
                osrfHashRemove( class->nodes, remoteId );
                if( osrfHashGetCount(class->nodes) == 0 ) {
                        osrfRouterRemoveClass( router, classname );
-                       return 1;
                }
-
-               return 0;
        }
-
-       return -1;
 }
 
 
 /**
        @brief Free a router class object.
-       @param classname Class name.
+       @param classname Class name (not used).
        @param c Pointer to the osrfRouterClass, cast to a void pointer.
 
-       This function is designated to the osrfHash as a callback.
+       This function is invoked as a callback when we remove an osrfRouterClass from the
+       router's list of classes.
 */
 static void osrfRouterClassFree( char* classname, void* c ) {
-       if(!(classname && c)) return;
+       if( !c )
+               return;
        osrfRouterClass* rclass = (osrfRouterClass*) c;
        client_disconnect( rclass->connection );
        client_free( rclass->connection );
@@ -631,7 +654,7 @@ static void osrfRouterClassFree( char* classname, void* c ) {
 
 /**
        @brief Free an osrfRouterNode.
-       @param remoteId Name of router (not used).
+       @param remoteId Jabber ID of node (not used).
        @param n Pointer to the osrfRouterNode to be freed, cast to a void pointer.
 
        This is a callback installed in an osrfHash (the nodes member of an osrfRouterClass).
@@ -645,6 +668,13 @@ static void osrfRouterNodeFree( char* remoteId, void* n ) {
 }
 
 
+/**
+       @brief Free an osrfRouter and everything it owns.
+       @param router Pointer to the osrfRouter to be freed.
+
+       The osrfRouterClasses and osrfRouterNodes are freed by callback functions installed in
+       the osrfHashes.
+*/
 void osrfRouterFree( osrfRouter* router ) {
        if(!router) return;
 
@@ -657,15 +687,18 @@ void osrfRouterFree( osrfRouter* router ) {
 
        osrfStringArrayFree( router->trustedClients );
        osrfStringArrayFree( router->trustedServers );
+       osrfListFree( router->message_list );
 
        client_free( router->connection );
        free(router);
 }
 
 
-
-/*
-       Finds the class associated with the given class name in the list of classes
+/**
+       @brief Given a class name, find the corresponding osrfRouterClass.
+       @param router Pointer to the osrfRouter that owns the osrfRouterClass.
+       @param classname Name of the class.
+       @return Pointer to a matching osrfRouterClass if found, or NULL if not.
 */
 static osrfRouterClass* osrfRouterFindClass( osrfRouter* router, const char* classname ) {
        if(!( router && router->classes && classname )) return NULL;
@@ -673,8 +706,11 @@ static osrfRouterClass* osrfRouterFindClass( osrfRouter* router, const char* cla
 }
 
 
-/*
-       Finds the router node within this class with the given remote id
+/**
+       @brief Find a given node for a given class.
+       @param rclass Pointer to the osrfRouterClass in which to search.
+       @param remoteId Jabber ID of the node for which to search.
+       @return Pointer to the matching osrfRouterNode, if found; otherwise NULL.
 */
 static osrfRouterNode* osrfRouterClassFindNode( osrfRouterClass* rclass,
                const char* remoteId ) {
@@ -683,12 +719,6 @@ static osrfRouterNode* osrfRouterClassFindNode( osrfRouterClass* rclass,
 }
 
 
-/*
-       Clears and populates the provided fd_set* with file descriptors
-       from the router's top level connection as well as each of the
-       router class connections
-       @return The largest file descriptor found in the filling process
-*/
 /**
        @brief Fill an fd_set with all the sockets owned by the osrfRouter.
        @param router Pointer to the osrfRouter whose sockets are to be used.
@@ -708,7 +738,8 @@ static int _osrfRouterFillFDSet( osrfRouter* router, fd_set* set ) {
        int sockid;
 
        osrfRouterClass* class = NULL;
-       osrfHashIterator* itr = osrfNewHashIterator(router->classes);
+       osrfHashIterator* itr = router->class_itr;
+       osrfHashIteratorReset( itr );
 
        while( (class = osrfHashIteratorNext(itr)) ) {
                const char* classname = osrfHashIteratorKey(itr);
@@ -730,84 +761,125 @@ static int _osrfRouterFillFDSet( osrfRouter* router, fd_set* set ) {
                }
        }
 
-       osrfHashIteratorFree(itr);
        return maxfd;
 }
 
-/*
-       handles messages that don't have a 'router_command' set.  They are assumed to
-       be app request messages
+/**
+       @brief Handler a router-level message that isn't a command; presumed to be an app request.
+       @param router Pointer to the current osrfRouter.
+       @param msg Pointer to the incoming message.
+
+       The body of the transport_message is a JSON string, specifically an JSON array.
+       Translate the JSON into a series of osrfMessages, one for each element of the JSON
+       array.  Process each osrfMessage in turn.  Each message is either a CONNECT or a
+       REQUEST.
 */
-static void osrfRouterHandleAppRequest( osrfRouter* router, transport_message* msg ) {
-
-       int T = 32;
-       osrfMessage* arr[T];
-       memset(arr, 0, sizeof(arr));
+static void osrfRouterHandleAppRequest( osrfRouter* router, const transport_message* msg ) {
 
-       int num_msgs = osrf_message_deserialize( msg->body, arr, T );
+       // Translate the JSON into a list of osrfMessages
+       router->message_list = osrfMessageDeserialize( msg->body, router->message_list );
        osrfMessage* omsg = NULL;
 
+       // Process each osrfMessage
        int i;
-       for( i = 0; i != num_msgs; i++ ) {
+       for( i = 0; i < router->message_list->size; ++i ) {
 
-               if( !(omsg = arr[i]) ) continue;
+               omsg = osrfListGetIndex( router->message_list, i );
+               if( omsg ) {
 
-               switch( omsg->m_type ) {
+                       switch( omsg->m_type ) {
 
-                       case CONNECT:
-                               osrfRouterRespondConnect( router, msg, omsg );
-                               break;
+                               case CONNECT:
+                                       osrfRouterRespondConnect( router, msg, omsg );
+                                       break;
 
-                       case REQUEST:
-                               osrfRouterProcessAppRequest( router, msg, omsg );
-                               break;
+                               case REQUEST:
+                                       osrfRouterProcessAppRequest( router, msg, omsg );
+                                       break;
 
-                       default: break;
-               }
+                               default:
+                                       break;
+                       }
 
-               osrfMessageFree( omsg );
+                       osrfMessageFree( omsg );
+               }
        }
 
        return;
 }
 
-static int osrfRouterRespondConnect( osrfRouter* router, transport_message* msg,
-               osrfMessage* omsg ) {
-       if(!(router && msg && omsg)) return -1;
+/**
+       @brief Respond to a CONNECT message.
+       @param router Pointer to the current osrfRouter.
+       @param msg Pointer to the transport_message that the osrfMessage came from.
+       @param omsg Pointer to the osrfMessage to be processed.
 
-       osrfMessage* success = osrf_message_init( STATUS, omsg->thread_trace, omsg->protocol );
+       An application is trying to connect to the router.  Reply with a STATUS message
+       signifying success.
+
+       "CONNECT" is a bit of a misnomer.  We don't establish a stateful session; i.e. we
+       don't retain any memory of this message.  We just confirm that the router is alive,
+       and that the client has a good address for it.
+*/
+static void osrfRouterRespondConnect( osrfRouter* router, const transport_message* msg,
+               const osrfMessage* omsg ) {
+       if(!(router && msg && omsg))
+               return;
 
        osrfLogDebug( OSRF_LOG_MARK, "router received a CONNECT message from %s", msg->sender );
 
+       // Build a success message
+       osrfMessage* success = osrf_message_init( STATUS, omsg->thread_trace, omsg->protocol );
        osrf_message_set_status_info(
                success, "osrfConnectStatus", "Connection Successful", OSRF_STATUS_OK );
 
+       // Translate the success message into JSON,
+       // and then package the JSON in a transport message
        char* data = osrf_message_serialize(success);
-
-       transport_message* return_m = message_init(
-               data, "", msg->thread, msg->sender, "" );
-
-       client_send_message(router->connection, return_m);
-
-       free(data);
-       osrfMessageFree(success);
-       message_free(return_m);
-
-       return 0;
+       osrfMessageFree( success );
+       transport_message* return_msg = message_init(
+               data,           // message payload, translated from osrfMessage
+               "",             // no subject
+               msg->thread,    // same thread
+               msg->sender,    // destination (client's Jabber ID)
+               ""              // don't send our address; client already has it
+       );
+       free( data );
+
+       client_send_message( router->connection, return_msg );
+       message_free( return_msg );
 }
 
 
+/**
+       @brief Respond to a REQUEST message.
+       @param router Pointer to the current osrfRouter.
+       @param msg Pointer to the transport_message that the osrfMessage came from.
+       @param omsg Pointer to the osrfMessage to be processed.
+
+       Respond to an information request from an application.  Most types of request involve
+       one or more counts of messages successfully routed.
+
+       Request types currently supported:
+       - "opensrf.router.info.class.list" -- list of class names.
+       - "opensrf.router.info.stats.class.summary" -- total count for a specified class.
+       - "opensrf.router.info.stats.class" -- count for every node of a specified class.
+       - "opensrf.router.info.stats.class.all" -- count for every node of every class.
+       - "opensrf.router.info.stats.class.node.all" -- total count for every class.
+*/
+static void osrfRouterProcessAppRequest( osrfRouter* router, const transport_message* msg,
+               const osrfMessage* omsg ) {
 
-static int osrfRouterProcessAppRequest( osrfRouter* router, transport_message* msg,
-               osrfMessage* omsg ) {
-
-       if(!(router && msg && omsg && omsg->method_name)) return -1;
+       if(!(router && msg && omsg && omsg->method_name))
+               return;
 
        osrfLogInfo( OSRF_LOG_MARK, "Router received app request: %s", omsg->method_name );
 
+       // Branch on the request type.  Build a jsonObject as an answer to the request.
        jsonObject* jresponse = NULL;
        if(!strcmp( omsg->method_name, ROUTER_REQUEST_CLASS_LIST )) {
 
+               // Prepare an array of class names.
                int i;
                jresponse = jsonNewObjectType(JSON_ARRAY);
 
@@ -816,25 +888,25 @@ static int osrfRouterProcessAppRequest( osrfRouter* router, transport_message* m
                        jsonObjectPush( jresponse, jsonNewObject(osrfStringArrayGetString( keys, i )) );
                osrfStringArrayFree(keys);
 
-
        } else if(!strcmp( omsg->method_name, ROUTER_REQUEST_STATS_CLASS_SUMMARY )) {
 
-               osrfRouterClass* class;
-               osrfRouterNode* node;
+               // Prepare a count of all the messages successfully routed for a given class.
                int count = 0;
 
-               char* classname = jsonObjectToSimpleString( jsonObjectGetIndex( omsg->_params, 0 ) );
-
+               // class name is the first parameter
+               const char* classname = jsonObjectGetString( jsonObjectGetIndex( omsg->_params, 0 ) );
                if (!classname)
-                       return -1;
+                       return;
 
-               class = osrfHashGet(router->classes, classname);
-               free(classname);
+               osrfRouterClass* class = osrfHashGet(router->classes, classname);
 
+               // For each node: add the count to the total.
+               osrfRouterNode* node;
                osrfHashIterator* node_itr = osrfNewHashIterator(class->nodes);
                while( (node = osrfHashIteratorNext(node_itr)) ) {
                        count += node->count;
-                       //jsonObjectSetKey( class_res, node->remoteId, jsonNewNumberObject( (double) node->count ) );
+                       // jsonObjectSetKey( class_res, node->remoteId, 
+                       //       jsonNewNumberObject( (double) node->count ) );
                }
                osrfHashIteratorFree(node_itr);
 
@@ -842,18 +914,19 @@ static int osrfRouterProcessAppRequest( osrfRouter* router, transport_message* m
 
        } else if(!strcmp( omsg->method_name, ROUTER_REQUEST_STATS_CLASS )) {
 
-               osrfRouterClass* class;
-               osrfRouterNode* node;
-
-               char* classname = jsonObjectToSimpleString( jsonObjectGetIndex( omsg->_params, 0 ) );
+               // Prepare a hash for a given class.  Key: the remoteId of a node.  Datum: the
+               // number of messages successfully routed for that node.
 
+               // class name is the first parameter
+               const char* classname = jsonObjectGetString( jsonObjectGetIndex( omsg->_params, 0 ) );
                if (!classname)
-                       return -1;
+                       return;
 
                jresponse = jsonNewObjectType(JSON_HASH);
-               class = osrfHashGet(router->classes, classname);
-               free(classname);
+               osrfRouterClass* class = osrfHashGet(router->classes, classname);
 
+               // For each node: get the count and store it in the hash.
+               osrfRouterNode* node;
                osrfHashIterator* node_itr = osrfNewHashIterator(class->nodes);
                while( (node = osrfHashIteratorNext(node_itr)) ) {
                        jsonObjectSetKey( jresponse, node->remoteId,
@@ -863,16 +936,20 @@ static int osrfRouterProcessAppRequest( osrfRouter* router, transport_message* m
 
        } else if(!strcmp( omsg->method_name, ROUTER_REQUEST_STATS_CLASS_FULL )) {
 
+               // Prepare a hash of hashes, giving the message counts for each node for each class.
+
                osrfRouterClass* class;
                osrfRouterNode* node;
-               jresponse = jsonNewObjectType(JSON_HASH);
+               jresponse = jsonNewObjectType(JSON_HASH);  // Key: class name.
 
+               // Traverse the list of classes.
                osrfHashIterator* class_itr = osrfNewHashIterator(router->classes);
                while( (class = osrfHashIteratorNext(class_itr)) ) {
 
-                       jsonObject* class_res = jsonNewObjectType(JSON_HASH);
+                       jsonObject* class_res = jsonNewObjectType(JSON_HASH);  // Key: remoteId of node.
                        const char* classname = osrfHashIteratorKey(class_itr);
 
+                       // Traverse the list of nodes for the current class.
                        osrfHashIterator* node_itr = osrfNewHashIterator(class->nodes);
                        while( (node = osrfHashIteratorNext(node_itr)) ) {
                                jsonObjectSetKey( class_res, node->remoteId,
@@ -887,19 +964,21 @@ static int osrfRouterProcessAppRequest( osrfRouter* router, transport_message* m
 
        } else if(!strcmp( omsg->method_name, ROUTER_REQUEST_STATS_NODE_FULL )) {
 
+               // Prepare a hash. Key: class name.  Datum: total number of successfully routed
+               // messages routed for nodes of that class.
+
                osrfRouterClass* class;
                osrfRouterNode* node;
-               int count;
                jresponse = jsonNewObjectType(JSON_HASH);
 
                osrfHashIterator* class_itr = osrfNewHashIterator(router->classes);
-               while( (class = osrfHashIteratorNext(class_itr)) ) {
+               while( (class = osrfHashIteratorNext(class_itr)) ) {  // For each class
 
-                       count = 0;
+                       int count = 0;
                        const char* classname = osrfHashIteratorKey(class_itr);
 
                        osrfHashIterator* node_itr = osrfNewHashIterator(class->nodes);
-                       while( (node = osrfHashIteratorNext(node_itr)) ) {
+                       while( (node = osrfHashIteratorNext(node_itr)) ) {  // For each node
                                count += node->count;
                        }
                        osrfHashIteratorFree(node_itr);
@@ -909,84 +988,91 @@ static int osrfRouterProcessAppRequest( osrfRouter* router, transport_message* m
 
                osrfHashIteratorFree(class_itr);
 
-       } else {
+       } else {  // None of the above
 
-               return osrfRouterHandleMethodNFound( router, msg, omsg );
+               osrfRouterHandleMethodNFound( router, msg, omsg );
+               return;
        }
 
-
-       osrfRouterHandleAppResponse( router, msg, omsg, jresponse );
+       // Send the result back to the requester.
+       osrfRouterSendAppResponse( router, msg, omsg, jresponse );
        jsonObjectFree(jresponse);
-
-       return 0;
-
 }
 
 
-static int osrfRouterHandleMethodNFound(
-               osrfRouter* router, transport_message* msg, osrfMessage* omsg ) {
-
-       osrfMessage* err = osrf_message_init( STATUS, omsg->thread_trace, 1);
-               osrf_message_set_status_info( err,
-                               "osrfMethodException", "Router method not found", OSRF_STATUS_NOTFOUND );
+/**
+       @brief Respond to an invalid REQUEST message.
+       @param router Pointer to the current osrfRouter.
+       @param msg Pointer to the transport_message that contained the REQUEST message.
+       @param omsg Pointer to the osrfMessage that contained the REQUEST.
+*/
+static void osrfRouterHandleMethodNFound( osrfRouter* router,
+               const transport_message* msg, const osrfMessage* omsg ) {
 
-               char* data =  osrf_message_serialize(err);
+       // Create an exception message
+       osrfMessage* err = osrf_message_init( STATUS, omsg->thread_trace, 1 );
+       osrf_message_set_status_info( err,
+                       "osrfMethodException", "Router method not found", OSRF_STATUS_NOTFOUND );
 
-               transport_message* tresponse = message_init(
-                               data, "", msg->thread, msg->sender, msg->recipient );
+       // Translate it into JSON
+       char* data =  osrf_message_serialize(err);
+       osrfMessageFree( err );
 
-               client_send_message(router->connection, tresponse );
+       // Wrap the JSON up in a transport_message
+       transport_message* tresponse = message_init(
+                       data, "", msg->thread, msg->sender, msg->recipient );
+       free(data);
 
-               free(data);
-               osrfMessageFree( err );
-               message_free(tresponse);
-               return 0;
+       // Send it
+       client_send_message( router->connection, tresponse );
+       message_free( tresponse );
 }
 
 
-static int osrfRouterHandleAppResponse( osrfRouter* router,
-       transport_message* msg, osrfMessage* omsg, const jsonObject* response ) {
+/**
+       @brief Send a response to a router REQUEST message.
+       @param router Pointer to the current osrfRouter.
+       @param msg Pointer to the transport_message that contained the REQUEST message.
+       @param omsg Pointer to the osrfMessage that contained the REQUEST.
+       @param response Pointer to the jsonObject that constitutes the response.
+*/
+static void osrfRouterSendAppResponse( osrfRouter* router, const transport_message* msg,
+               const osrfMessage* omsg, const jsonObject* response ) {
 
        if( response ) { /* send the response message */
 
+               // Turn the jsonObject into JSON, and load it into an osrfMessage
                osrfMessage* oresponse = osrf_message_init(
                                RESULT, omsg->thread_trace, omsg->protocol );
 
                char* json = jsonObjectToJSON(response);
-               osrf_message_set_result_content( oresponse, json);
+               osrf_message_set_result_content( oresponse, json );
+               free(json);
 
+               // Package the osrfMessage into a transport_message, and send it
                char* data =  osrf_message_serialize(oresponse);
+               osrfMessageFree(oresponse);
                osrfLogDebug( OSRF_LOG_MARK,  "Responding to client app request with data: \n%s\n", data );
 
                transport_message* tresponse = message_init(
                                data, "", msg->thread, msg->sender, msg->recipient );
+               free(data);
 
                client_send_message(router->connection, tresponse );
-
-               osrfMessageFree(oresponse);
                message_free(tresponse);
-               free(json);
-               free(data);
        }
 
        /* now send the 'request complete' message */
        osrfMessage* status = osrf_message_init( STATUS, omsg->thread_trace, 1);
        osrf_message_set_status_info( status, "osrfConnectStatus", "Request Complete",
                        OSRF_STATUS_COMPLETE );
-
        char* statusdata = osrf_message_serialize(status);
+       osrfMessageFree(status);
 
        transport_message* sresponse = message_init(
                        statusdata, "", msg->thread, msg->sender, msg->recipient );
-       client_send_message(router->connection, sresponse );
-
        free(statusdata);
-       osrfMessageFree(status);
-       message_free(sresponse);
 
-       return 0;
+       client_send_message(router->connection, sresponse );
+       message_free(sresponse);
 }
-
-
-
-