[Opensrf-commits] r1953 - in trunk: bin include/opensrf src/libopensrf (scottmk)

svn at svn.open-ils.org svn at svn.open-ils.org
Tue May 4 09:41:17 EDT 2010


Author: scottmk
Date: 2010-05-04 09:41:16 -0400 (Tue, 04 May 2010)
New Revision: 1953

Modified:
   trunk/bin/osrf_ctl.sh.in
   trunk/include/opensrf/osrf_system.h
   trunk/include/opensrf/utils.h
   trunk/src/libopensrf/opensrf.c
   trunk/src/libopensrf/osrf_system.c
   trunk/src/libopensrf/utils.c
Log:
Fix a bug that occasionally caused OSRF not to shut down cleanly.

The osrf_ctl.sh script had been using ps + grep to capture
the process ID (PID) of the opensrf-c daemon so that it could
send a SIGINT signal to it later to shut it down.  However the
script was also capturing the PIDs of the daemon's child processes
(i.e. the listener processes), which hadn't yet changed to
application-specific names.

As a result, when shutting down, the listener processes would
receive signals from two different sources: from the opensrf-c
daemon and from the surrounding shell script.  If the signal
from opensrf-c got there first, the kill from the script would
fail, and the script would abort, even though the process had
been successfully killed.

The solution is for opensrf.c to write the daemon's PID directly
to a file, instead of relying on ps + grep to capture it.  The
file name is specified by an additional command line parameter,
which (for upward compatibility) is currently optional.

Because this change involves a change to the osrf_ctl.sh
script, it will be necessary to run configure before the
usual make and make install.  If you are using the usual
configuration, run the following from within the OSRF
trunk directory:

./configure --prefix=/openils --sysconfdir=/openils/conf

If you don't run configure, the old osrf_ctl.sh script will
continue to work as it has in the past, and you won't get
the benefit of the change.

M    include/opensrf/utils.h
M    include/opensrf/osrf_system.h
M    src/libopensrf/utils.c
M    src/libopensrf/opensrf.c
M    src/libopensrf/osrf_system.c
M    bin/osrf_ctl.sh.in


Modified: trunk/bin/osrf_ctl.sh.in
===================================================================
--- trunk/bin/osrf_ctl.sh.in	2010-05-03 02:23:59 UTC (rev 1952)
+++ trunk/bin/osrf_ctl.sh.in	2010-05-04 13:41:16 UTC (rev 1953)
@@ -174,10 +174,7 @@
 	fi;
 
 	do_action "start" $PID_OSRF_C "OpenSRF C (host=$host)";
-	opensrf-c $host $OPT_CONFIG opensrf;
-    sleep 1; # give the main C proc time to appear in ps
-	pid=$(ps ax | grep "OpenSRF System-C" | grep -v grep | awk '{print $1}')
-	echo $pid > "$PID_OSRF_C";
+	opensrf-c $host $OPT_CONFIG opensrf "$PID_OSRF_C";
 	return 0;
 }
 

Modified: trunk/include/opensrf/osrf_system.h
===================================================================
--- trunk/include/opensrf/osrf_system.h	2010-05-03 02:23:59 UTC (rev 1952)
+++ trunk/include/opensrf/osrf_system.h	2010-05-04 13:41:16 UTC (rev 1953)
@@ -17,6 +17,8 @@
 extern "C" {
 #endif
 
+void osrfSystemSetPidFile( const char* name );
+
 int osrf_system_bootstrap_client( char* config_file, char* contextnode );
 
 int osrfSystemBootstrapClientResc( const char* config_file,

Modified: trunk/include/opensrf/utils.h
===================================================================
--- trunk/include/opensrf/utils.h	2010-05-03 02:23:59 UTC (rev 1952)
+++ trunk/include/opensrf/utils.h	2010-05-04 13:41:16 UTC (rev 1953)
@@ -280,8 +280,8 @@
 int init_proc_title( int argc, char* argv[] );
 int set_proc_title( const char* format, ... );
 
-
 int daemonize( void );
+int daemonize_write_pid( FILE* pidfile );
 
 void* safe_malloc(int size);
 void* safe_calloc(int size);

Modified: trunk/src/libopensrf/opensrf.c
===================================================================
--- trunk/src/libopensrf/opensrf.c	2010-05-03 02:23:59 UTC (rev 1952)
+++ trunk/src/libopensrf/opensrf.c	2010-05-04 13:41:16 UTC (rev 1953)
@@ -1,5 +1,17 @@
-#include <opensrf/osrf_system.h>
+#include "opensrf/osrf_system.h"
 
+/**
+	@brief Run an OSRF server as defined by the command line and a config file.
+	@param argc Number of command line arguments, plus one.
+	@param argv Ragged array of command name plus command line arguments.
+	@return 0 if successful, or 1 if failure.
+
+	Command line parameters:
+	- Full network name of the host where the process is running; or 'localhost' will do.
+	- Name of the configuration file; normally '/openils/conf/opensrf_core.xml'.
+	- Name of an aggregate within the configuration file, containing the relevant subset
+	of configuration stuff.
+*/
 int main( int argc, char* argv[] ) {
 
 	if( argc < 4 ) {
@@ -7,12 +19,15 @@
 		return 1;
 	}
 
-	/* these must be strdup'ed because init_proc_title / set_proc_title 
+	/* these must be strdup'ed because init_proc_title / set_proc_title
 		are evil and overwrite the argv memory */
-	char* host		= strdup( argv[1] );
-	char* config	= strdup( argv[2] );
-	char* context	= strdup( argv[3] );
+	char* host      = strdup( argv[1] );
+	char* config    = strdup( argv[2] );
+	char* context   = strdup( argv[3] );
 
+	if( argv[4] )
+		osrfSystemSetPidFile( argv[4] );
+
 	init_proc_title( argc, argv );
 	set_proc_title( "OpenSRF System-C" );
 
@@ -26,7 +41,6 @@
 		);
 	}
 
-
 	free(host);
 	free(config);
 	free(context);

Modified: trunk/src/libopensrf/osrf_system.c
===================================================================
--- trunk/src/libopensrf/osrf_system.c	2010-05-03 02:23:59 UTC (rev 1952)
+++ trunk/src/libopensrf/osrf_system.c	2010-05-04 13:41:16 UTC (rev 1953)
@@ -51,6 +51,9 @@
 /** Boolean: set to true when we finish shutting down. */
 static int shutdownComplete = 0;
 
+/** Name of file to which to write the process ID of the child process */
+char* pidfile_name = NULL;
+
 static void add_child( pid_t pid, const char* app, const char* libfile );
 static void delete_child( ChildNode* node );
 static void delete_all_children( void );
@@ -122,6 +125,27 @@
 }
 
 /**
+	@brief Save a copy of a file name to be used for writing a process ID.
+	@param name Designated file name, or NULL.
+
+	Save a file name for later use in saving a process ID.  If @a name is NULL, leave
+	the file name NULL.
+
+	When the parent process spawns a child, the child becomes a daemon.  The parent writes the
+	child's process ID to the PID file, if one has been designated, so that some other process
+	can retrieve the PID later and kill the daemon.
+*/
+void osrfSystemSetPidFile( const char* name ) {
+	if( pidfile_name )
+		free( pidfile_name );
+
+	if( name )
+		pidfile_name = strdup( name );
+	else
+		pidfile_name = NULL;
+}
+
+/**
 	@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
@@ -222,7 +246,23 @@
 	// Turn into a daemon.  The parent forks and exits.  Only the
 	// child returns, with the standard streams (stdin, stdout, and
 	// stderr) redirected to /dev/null.
-	daemonize();
+	FILE* pidfile = NULL;
+	if( pidfile_name ) {
+		pidfile = fopen( pidfile_name, "w" );
+		if( !pidfile ) {
+			osrfLogError( OSRF_LOG_MARK, "Unable to open PID file \"%s\": %s",
+				pidfile_name, strerror( errno ) );
+			free( pidfile_name );
+			pidfile_name = NULL;
+			return -1;
+		}
+	}
+	daemonize_write_pid( pidfile );
+	if( pidfile ) {
+		fclose( pidfile );
+		free( pidfile_name );
+		pidfile_name = NULL;
+	}
 
 	jsonObject* apps = osrf_settings_host_value_object("/activeapps/appname");
 	osrfStringArray* arr = osrfNewStringArray(8);
@@ -588,8 +628,6 @@
 	snprintf(buf, len - 1, "%s_%s_%s_%ld", resource, host, tbuf, (long) getpid() );
 
 	if(client_connect( client, username, password, buf, 10, AUTH_DIGEST )) {
-		/* child nodes will leak the parents client... but we can't free
-			it without disconnecting the parents client :( */
 		osrfGlobalTransportClient = client;
 	}
 

Modified: trunk/src/libopensrf/utils.c
===================================================================
--- trunk/src/libopensrf/utils.c	2010-05-03 02:23:59 UTC (rev 1952)
+++ trunk/src/libopensrf/utils.c	2010-05-04 13:41:16 UTC (rev 1953)
@@ -647,11 +647,22 @@
 	@brief Become a proper daemon.
 	@return 0 if successful, or -1 if not.
 
-	Call fork().  The parent exits.  The child moves to the root
-	directory, detaches from the terminal, and redirects the
-	standard streams (stdin, stdout, stderr) to /dev/null.
+	Call fork().  The parent exits.  The child moves to the root directory, detaches from
+	the terminal, and redirects the standard streams (stdin, stdout, stderr) to /dev/null.
 */
 int daemonize( void ) {
+	return daemonize_write_pid( NULL );
+}
+/**
+	@brief Become a proper daemon, and report the childs process ID.
+	@return 0 if successful, or -1 if not.
+
+	Call fork().  If pidfile is not NULL, the parent writes the process ID of the child
+	process to the specified file.  Then it exits.  The child moves to the root
+	directory, detaches from the terminal, and redirects the standard streams (stdin,
+	stdout, stderr) to /dev/null.
+ */
+int daemonize_write_pid( FILE* pidfile ) {
 	pid_t f = fork();
 
 	if (f == -1) {
@@ -677,6 +688,10 @@
 		return 0;
 
 	} else { // We're in the parent...
+		if( pidfile ) {
+			fprintf( pidfile, "%ld\n", (long) f );
+			fclose( pidfile );
+		}
 		_exit(0);
 	}
 }



More information about the opensrf-commits mailing list