From 779e7c48a1e7fdb51cf2d67ac1cd5fd3ca50b5f1 Mon Sep 17 00:00:00 2001 From: scottmk Date: Fri, 15 Jan 2010 04:52:00 +0000 Subject: [PATCH] Close a substantial resource leak in drone processes. 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 | 2 ++ include/opensrf/transport_session.h | 2 ++ src/libopensrf/osrf_system.c | 27 ++++++++++++++++++++--- src/libopensrf/transport_client.c | 23 ++++++++++++++++--- src/libopensrf/transport_session.c | 34 +++++++++++++++++++++++------ 5 files changed, 75 insertions(+), 13 deletions(-) diff --git a/include/opensrf/transport_client.h b/include/opensrf/transport_client.h index 4d9dcc7..4307af9 100644 --- a/include/opensrf/transport_client.h +++ b/include/opensrf/transport_client.h @@ -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 ); diff --git a/include/opensrf/transport_session.h b/include/opensrf/transport_session.h index 4b4b921..01d7ed1 100644 --- a/include/opensrf/transport_session.h +++ b/include/opensrf/transport_session.h @@ -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, diff --git a/src/libopensrf/osrf_system.c b/src/libopensrf/osrf_system.c index be805c1..bd22fdc 100644 --- a/src/libopensrf/osrf_system.c +++ b/src/libopensrf/osrf_system.c @@ -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; } diff --git a/src/libopensrf/transport_client.c b/src/libopensrf/transport_client.c index c6c3520..79f0aa6 100644 --- a/src/libopensrf/transport_client.c +++ b/src/libopensrf/transport_client.c @@ -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; diff --git a/src/libopensrf/transport_session.c b/src/libopensrf/transport_session.c index 183d976..c4dc4c8 100644 --- a/src/libopensrf/transport_session.c +++ b/src/libopensrf/transport_session.c @@ -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. -- 2.43.2