Close a substantial resource leak in drone processes.
authorscottmk <scottmk@9efc2488-bf62-4759-914b-345cdb29e865>
Fri, 15 Jan 2010 04:52:00 +0000 (04:52 +0000)
committerscottmk <scottmk@9efc2488-bf62-4759-914b-345cdb29e865>
Fri, 15 Jan 2010 04:52:00 +0000 (04:52 +0000)
A drone inherits the transport_client of its parent process,
including a socket and a substantial amount of memory.  The
old code avoided freeing the transport_client in order to
avoid disconnecting the parent from Jabber.

The new code contrives to reclaim the resources without
sending a disconnect to Jabber.  Hence the parent remains
connected.

M    include/opensrf/transport_client.h
M    include/opensrf/transport_session.h
M    src/libopensrf/osrf_system.c
M    src/libopensrf/transport_session.c
M    src/libopensrf/transport_client.c

git-svn-id: svn://svn.open-ils.org/OpenSRF/trunk@1890 9efc2488-bf62-4759-914b-345cdb29e865

include/opensrf/transport_client.h
include/opensrf/transport_session.h
src/libopensrf/osrf_system.c
src/libopensrf/transport_client.c
src/libopensrf/transport_session.c

index 4d9dcc7..4307af9 100644 (file)
@@ -46,6 +46,8 @@ int client_disconnect( transport_client* client );
 
 int client_free( transport_client* client );
 
+int client_discard( transport_client* client );
+
 int client_send_message( transport_client* client, transport_message* msg );
 
 int client_connected( const transport_client* client );
index 4b4b921..01d7ed1 100644 (file)
@@ -96,6 +96,8 @@ int session_connected( transport_session* session );
 
 int session_free( transport_session* session );
 
+int session_discard( transport_session* session );
+
 int session_connect( transport_session* session,
                const char* username, const char* password,
                const char* resource, int connect_timeout,
index be805c1..bd22fdc 100644 (file)
@@ -34,15 +34,19 @@ static void handleKillSignal(int signo) {
        _exit(0);
 }
 
+/**
+       @brief Represents a child process.
+*/
 struct child_node
 {
-       ChildNode* pNext;
-       ChildNode* pPrev;
-       pid_t pid;
+       ChildNode* pNext;  /**< Linkage pointer for doubly linked list. */
+       ChildNode* pPrev;  /**< Linkage pointer for doubly linked list. */
+       pid_t pid;         /**< Process ID of the child process. */
        char* app;
        char* libfile;
 };
 
+/** List of child processes. */
 static ChildNode* child_list;
 
 /** Pointer to the global transport_client; i.e. our connection to Jabber. */
@@ -53,11 +57,28 @@ static void delete_child( ChildNode* node );
 static void delete_all_children( void );
 static ChildNode* seek_child( pid_t pid );
 
+/**
+       @brief Return a pointer to the global transport_client.
+       @return Pointer to the global transport_client, or NULL.
+
+       A given process needs only one connection to Jabber, so we keep it a pointer to it at
+       file scope.  This function returns that pointer.
+
+       If the connection has been opened by a previous call to osrfSystemBootstrapClientResc(),
+       Return the pointer.  Otherwise return NULL.
+*/
 transport_client* osrfSystemGetTransportClient( void ) {
        return osrfGlobalTransportClient;
 }
 
+/**
+       @brief Discard the global transport_client, but without disconnecting from Jabber.
+
+       To be called by a child process in order to disregard the parent's connection without
+       disconnecting it, since disconnecting would disconnect the parent as well.
+*/
 void osrfSystemIgnoreTransportClient() {
+       client_discard( osrfGlobalTransportClient );
        osrfGlobalTransportClient = NULL;
 }
 
index c6c3520..79f0aa6 100644 (file)
@@ -299,10 +299,27 @@ static void client_message_handler( void* client, transport_message* msg ){
        @param client Pointer to the transport_client to be freed.
        @return 1 if successful, or 0 if not.  The only error condition is if @a client is NULL.
 */
-int client_free( transport_client* client ){
-       if(client == NULL) return 0;
-
+int client_free( transport_client* client ) {
+       if(client == NULL)
+               return 0;
        session_free( client->session );
+       client->session = NULL;
+       return client_discard( client );
+}
+
+/**
+       @brief Free a transport_client's resources, but without disconnecting.
+       @param client Pointer to the transport_client to be freed.
+       @return 1 if successful, or 0 if not.  The only error condition is if @a client is NULL.
+
+       A child process may call this in order to free the resources associated with the parent's
+       transport_client, but without disconnecting from Jabber, since disconnecting would
+       disconnect the parent as well.
+ */
+int client_discard( transport_client* client ) {
+       if(client == NULL)
+               return 0;
+       
        transport_message* current = client->msg_q_head;
        transport_message* next;
 
index 183d976..c4dc4c8 100644 (file)
@@ -201,19 +201,36 @@ transport_session* init_transport( const char* server,
        return session;
 }
 
-
 /**
-       @brief Destroy a transport_session, and close its socket.
+       @brief Disconnect from Jabber, destroy a transport_session, and close its socket.
        @param session Pointer to the transport_session to be destroyed.
        @return 1 if successful, or 0 if not.
 
        The only error condition is a NULL pointer argument.
 */
 int session_free( transport_session* session ) {
-       if( ! session ) { return 0; }
-
-       if( session->sock_id )
+       if( ! session )
+               return 0;
+       else {
                session_disconnect( session );
+               return session_discard( session );
+       }
+}
+
+/**
+       @brief Destroy a transport_session and close its socket, without disconnecting from Jabber.
+       @param session Pointer to the transport_session to be destroyed.
+       @return 1 if successful, or 0 if not.
+
+       This function may be called from a child process in order to free resources associated
+       with the parent's transport_session, but without sending a disconnect to Jabber (since
+       that would disconnect the parent).
+
+       The only error condition is a NULL pointer argument.
+ */
+int session_discard( transport_session* session ) {
+       if( ! session )
+               return 0;
 
        if(session->sock_mgr)
                socket_manager_free(session->sock_mgr);
@@ -292,7 +309,7 @@ int session_wait( transport_session* session, int timeout ) {
 }
 
 /**
-       @brief Wrap a message in XML and send it to Jabber.
+       @brief Convert a transport_message to XML and send it to Jabber.
        @param session Pointer to the transport_session.
        @param msg Pointer to a transport_message enclosing the message.
        @return 0 if successful, or -1 upon error.
@@ -323,11 +340,14 @@ int session_send_msg(
        @param auth_type An enum: either AUTH_PLAIN or AUTH_DIGEST (see notes).
        @return 1 if successful, or 0 upon error.
 
-       If @a connect_timeout is -1, wait indefinitely for input activity to appear.  If
+       If @a connect_timeout is -1, wait indefinitely for the Jabber server to respond.  If
        @a connect_timeout is zero, don't wait at all.  If @a timeout is positive, wait that
        number of seconds before timing out.  If @a connect_timeout has a negative value other
        than -1, the results are not well defined.
 
+       The value of @a connect_timeout applies to each of two stages in the logon procedure.
+       Hence the logon may take up to twice the amount of time indicated.
+
        If we connect as a Jabber component, we send the password as an SHA1 hash.  Otherwise
        we look at the @a auth_type.  If it's AUTH_PLAIN, we send the password as plaintext; if
        it's AUTH_DIGEST, we send it as a hash.