[asterisk-commits] tilghman: branch 1.6.1 r240502 - in /branches/1.6.1: ./ main/ utils/

[SVN commits to the Asterisk project]

Author: tilghman
Date: Fri Jan 15 15:44:41 2010
New Revision: 240502

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=240502
Log:
Merged revisions 240499-240500 via svnmerge from 
https://origsvn.digium.com/svn/asterisk/trunk

........
  r240499 | tilghman | 2010-01-15 15:40:14 -0600 (Fri, 15 Jan 2010) | 9 lines
  
  The previous attempt at using a pipe to guarantee astcanary shutdown did not work.
  We're revisiting the previous patch, albeit with a method that overcomes the
  prior criticism that it was not POSIX-compliant.
  (closes issue #16602)
   Reported by: frawd
   Patches: 
         20100114__issue16602.diff.txt uploaded by tilghman (license 14)
   Tested by: frawd
........
  r240500 | tilghman | 2010-01-15 15:42:36 -0600 (Fri, 15 Jan 2010) | 2 lines
  
  Oops, missed an include
........

Modified:
    branches/1.6.1/   (props changed)
    branches/1.6.1/main/asterisk.c
    branches/1.6.1/utils/astcanary.c

Propchange: branches/1.6.1/
------------------------------------------------------------------------------
Binary property 'trunk-merged' - no diff available.

Modified: branches/1.6.1/main/asterisk.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.6.1/main/asterisk.c?view=diff&rev=240502&r1=240501&r2=240502
==============================================================================
--- branches/1.6.1/main/asterisk.c (original)
+++ branches/1.6.1/main/asterisk.c Fri Jan 15 15:44:41 2010
@@ -260,7 +260,6 @@
 static pthread_t consolethread = AST_PTHREADT_NULL;
 static int canary_pid = 0;
 static char canary_filename[128];
-static int canary_pipe = -1;
 
 static char randompool[256];
 
@@ -3325,15 +3324,6 @@
 
 	/* Spawning of astcanary must happen AFTER the call to daemon(3) */
 	if (isroot && ast_opt_high_priority) {
-		int cpipe[2];
-
-		/* PIPE signal ensures that astcanary dies when Asterisk dies */
-		if (pipe(cpipe)) {
-			fprintf(stderr, "Unable to open pipe for canary process: %s\n", strerror(errno));
-			exit(1);
-		}
-		canary_pipe = cpipe[0];
-
 		snprintf(canary_filename, sizeof(canary_filename), "%s/alt.asterisk.canary.tweet.tweet.tweet", ast_config_AST_RUN_DIR);
 
 		/* Don't let the canary child kill Asterisk, if it dies immediately */
@@ -3341,18 +3331,17 @@
 
 		canary_pid = fork();
 		if (canary_pid == 0) {
-			char canary_binary[128], *lastslash;
+			char canary_binary[128], *lastslash, ppid[12];
 
 			/* Reset signal handler */
 			signal(SIGCHLD, SIG_DFL);
 			signal(SIGPIPE, SIG_DFL);
 
-			dup2(cpipe[1], 0);
-			close(cpipe[1]);
 			ast_close_fds_above_n(0);
 			ast_set_priority(0);
-
-			execlp("astcanary", "astcanary", canary_filename, (char *)NULL);
+			snprintf(ppid, sizeof(ppid), "%d", (int) getpid());
+
+			execlp("astcanary", "astcanary", canary_filename, ppid, (char *)NULL);
 
 			/* If not found, try the same path as used to execute asterisk */
 			ast_copy_string(canary_binary, argv[0], sizeof(canary_binary));
@@ -3365,12 +3354,11 @@
 			_exit(1);
 		} else if (canary_pid > 0) {
 			pthread_t dont_care;
-			close(cpipe[1]);
 			ast_pthread_create_detached(&dont_care, NULL, canary_thread, NULL);
 		}
 
 		/* Kill the canary when we exit */
-		atexit(canary_exit);
+		ast_register_atexit(canary_exit);
 	}
 
 	if (ast_event_init()) {

Modified: branches/1.6.1/utils/astcanary.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.6.1/utils/astcanary.c?view=diff&rev=240502&r1=240501&r2=240502
==============================================================================
--- branches/1.6.1/utils/astcanary.c (original)
+++ branches/1.6.1/utils/astcanary.c Fri Jan 15 15:44:41 2010
@@ -25,6 +25,7 @@
 #include <unistd.h>
 #include <stdlib.h>
 #include <string.h>
+#include <stdio.h>
 
 /*!\brief
  * At one time, canaries were carried along with coal miners down
@@ -87,9 +88,25 @@
 int main(int argc, char *argv[])
 {
 	int fd;
+	pid_t parent;
+
+	if (argc < 3) {
+		fprintf(stderr, "Usage: %s <monitor-filename> <ppid>\n", argv[0]);
+		exit(1);
+	}
+
 	/* Run at normal priority */
 	setpriority(PRIO_PROCESS, 0, 0);
-	for (;;) {
+
+	/*!\note
+	 * See http://www.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap03.html#tag_03_265
+	 * for a justification of this approach.  The PPID after the creator dies in Linux and
+	 * most other Unix-like systems will be 1, but this is not strictly the case.  The POSIX
+	 * specification allows it to be an implementation-defined system process.  However, it
+	 * most certainly will not be the original parent PID, which makes the following code
+	 * POSIX-compliant.
+	 */
+	for (parent = atoi(argv[2]); parent == getppid() ;) {
 		/* Update the modification times (checked from Asterisk) */
 		if (utime(argv[1], NULL)) {
 			/* Recreate the file if it doesn't exist */
@@ -108,7 +125,7 @@
 		sleep(5);
 	}
 
-	/* Never reached */
+	/* Exit when the parent dies */
 	return 0;
 }