From 779e7c48a1e7fdb51cf2d67ac1cd5fd3ca50b5f1 Mon Sep 17 00:00:00 2001
From: scottmk <scottmk@9efc2488-bf62-4759-914b-345cdb29e865>
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.11.0