[launchd-changes] [23690] branches/PR-5898404/launchd/src

[source_changes at macosforge.org]

Revision: 23690
          http://trac.macosforge.org/projects/launchd/changeset/23690
Author:   dsorresso at apple.com
Date:     2008-08-22 14:34:46 -0700 (Fri, 22 Aug 2008)
Log Message:
-----------
Added a few comments.
Put in a fix for an issue found during investigation of rdar://problem/6045086, which doesn't appear to be a launchd bug, so I'm just rolling the fix in here since it's simple enough.
Changed _vproc_transaction_begin() to use a compare-and-swap loop to enforce the assumption that the transaction count is one greater after increment and thus making the termination logic correct.

Modified Paths:
--------------
    branches/PR-5898404/launchd/src/launchd_core_logic.c
    branches/PR-5898404/launchd/src/launchd_runtime.c
    branches/PR-5898404/launchd/src/libvproc.c

Modified: branches/PR-5898404/launchd/src/launchd_core_logic.c
===================================================================
--- branches/PR-5898404/launchd/src/launchd_core_logic.c	2008-08-22 02:32:06 UTC (rev 23689)
+++ branches/PR-5898404/launchd/src/launchd_core_logic.c	2008-08-22 21:34:46 UTC (rev 23690)
@@ -336,7 +336,7 @@
 #define AUTO_PICK_LEGACY_LABEL (const char *)(~0)
 
 struct job_s {
-	kq_callback kqjob_callback;
+	kq_callback kqjob_callback; /* MUST be first element of this structure for benefit of launchd's run loop. */
 	LIST_ENTRY(job_s) sle;
 	LIST_ENTRY(job_s) pid_hash_sle;
 	LIST_ENTRY(job_s) label_hash_sle;
@@ -1054,6 +1054,11 @@
 		job_assumes(j, kevent_mod((uintptr_t)&j->semaphores, EVFILT_TIMER, EV_DELETE, 0, 0, j) != -1);
 	}
 
+	if( j->exit_timeout ) {
+		/* Not a big deal if this fails. It means that the timer's already been freed. */
+		kevent_mod((uintptr_t)&j->exit_timeout, EVFILT_TIMER, EV_DELETE, 0, 0, NULL);
+	}
+
 	kevent_mod((uintptr_t)j, EVFILT_TIMER, EV_DELETE, 0, 0, NULL);
 
 	LIST_REMOVE(j, sle);

Modified: branches/PR-5898404/launchd/src/launchd_runtime.c
===================================================================
--- branches/PR-5898404/launchd/src/launchd_runtime.c	2008-08-22 02:32:06 UTC (rev 23689)
+++ branches/PR-5898404/launchd/src/launchd_runtime.c	2008-08-22 21:34:46 UTC (rev 23690)
@@ -616,9 +616,15 @@
 			kevi = &kev[i];
 
 			if (kevi->filter) {
+/* Leave on for SnowLeopard development. We should really try and identify what bugs would 
+ * cause kevi->udata to be invalid.
+ */
 #if 1
 				Dl_info dli;
 
+				/* Check if kevi->udata was either malloc(3)ed or is a valid function pointer. 
+				 * If neither, it's probably an invalid pointer and we should log it. 
+				 */
 				if (launchd_assumes(malloc_size(kevi->udata) || dladdr(kevi->udata, &dli))) {
 #endif
 				runtime_ktrace(RTKT_LAUNCHD_BSD_KEVENT|DBG_FUNC_START, kevi->ident, kevi->filter, kevi->fflags);

Modified: branches/PR-5898404/launchd/src/libvproc.c
===================================================================
--- branches/PR-5898404/launchd/src/libvproc.c	2008-08-22 02:32:06 UTC (rev 23689)
+++ branches/PR-5898404/launchd/src/libvproc.c	2008-08-22 21:34:46 UTC (rev 23690)
@@ -96,8 +96,6 @@
 void
 _vproc_transaction_begin(void)
 {
-	typeof(vproc_shmem->vp_shmem_transaction_cnt) newval;
-
 	if (unlikely(vproc_shmem == NULL)) {
 		int po_r = pthread_once(&shmem_inited, vproc_shmem_init);
 		if (po_r != 0 || vproc_shmem == NULL) {
@@ -105,17 +103,20 @@
 		}
 	}
 
-	newval = __sync_add_and_fetch(&vproc_shmem->vp_shmem_transaction_cnt, 1);
-
-	if (unlikely(newval < 1)) {
-		if (vproc_shmem->vp_shmem_flags & VPROC_SHMEM_EXITING) {
-			raise(SIGKILL);
-			__crashreporter_info__ = "raise(SIGKILL) failed";
-		} else {
-			__crashreporter_info__ = "Unbalanced: vproc_transaction_begin()";
+	typeof(vproc_shmem->vp_shmem_transaction_cnt) old = 0;
+	do {
+		old = vproc_shmem->vp_shmem_transaction_cnt;
+		
+		if (unlikely(old < 0)) {
+			if (vproc_shmem->vp_shmem_flags & VPROC_SHMEM_EXITING) {
+				raise(SIGKILL);
+				__crashreporter_info__ = "raise(SIGKILL) failed";
+			} else {
+				__crashreporter_info__ = "Unbalanced: vproc_transaction_begin()";
+			}
+			abort();
 		}
-		abort();
-	}
+	} while( !__sync_bool_compare_and_swap(&vproc_shmem->vp_shmem_transaction_cnt, old, old + 1) );
 }
 
 size_t
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.macosforge.org/pipermail/launchd-changes/attachments/20080822/28540539/attachment.html