3933 contract adoption can race
authorJerry Jelinek <jerry.jelinek@joyent.com>
Tue, 29 Mar 2011 15:47:25 -0700
changeset 14261 c584b682c5c8
parent 14260 15e9457c12ec
child 14262 432cf6600cdd
3933 contract adoption can race Reviewed by: Bryan Cantrill <[email protected]> Reviewed by: Richard Lowe <[email protected]> Reviewed by: Garrett D'Amore <[email protected]> Approved by: Dan McDonald <[email protected]>
usr/src/uts/common/os/contract.c
--- a/usr/src/uts/common/os/contract.c	Sat Aug 03 16:27:57 2013 -0700
+++ b/usr/src/uts/common/os/contract.c	Tue Mar 29 15:47:25 2011 -0700
@@ -497,7 +497,7 @@
 	contract_t *parent = &p->p_ct_process->conp_contract;
 	int inherit = 0;
 
-	ASSERT(p == curproc);
+	VERIFY(p == curproc);
 
 	mutex_enter(&ct->ct_lock);
 
@@ -547,7 +547,7 @@
 
 	if (inherit) {
 		ct->ct_state = CTS_INHERITED;
-		ASSERT(ct->ct_regent == parent);
+		VERIFY(ct->ct_regent == parent);
 		contract_process_take(parent, ct);
 
 		/*
@@ -2063,8 +2063,8 @@
 {
 	ct_kevent_t *e, *first = NULL;
 
-	ASSERT(q->ctq_listno == CTEL_CONTRACT);
-	ASSERT(newq->ctq_listno == CTEL_PBUNDLE);
+	VERIFY(q->ctq_listno == CTEL_CONTRACT);
+	VERIFY(newq->ctq_listno == CTEL_PBUNDLE);
 
 	mutex_enter(&q->ctq_lock);
 	mutex_enter(&newq->ctq_lock);
@@ -2077,8 +2077,16 @@
 		if ((e->cte_flags & (CTE_INFO | CTE_ACK)) == 0) {
 			if (first == NULL)
 				first = e;
-			list_insert_tail(&newq->ctq_events, e);
-			cte_hold(e);
+			/*
+			 * It is possible for adoption to race with an owner's
+			 * cte_publish_all(); we must only enqueue events that
+			 * have not already been enqueued.
+			 */
+			if (!list_link_active((list_node_t *)
+			    ((uintptr_t)e + newq->ctq_events.list_offset))) {
+				list_insert_tail(&newq->ctq_events, e);
+				cte_hold(e);
+			}
 		}
 	}
 
@@ -2117,7 +2125,7 @@
 	int flags, stopper;
 	int start = 1;
 
-	ASSERT(MUTEX_HELD(&q->ctq_lock));
+	VERIFY(MUTEX_HELD(&q->ctq_lock));
 
 	for (e = list_head(&q->ctq_events); e != NULL; e = next) {
 		next = list_next(&q->ctq_events, e);
@@ -2227,13 +2235,24 @@
  * cte_publish_all.
  */
 static void
-cte_publish(ct_equeue_t *q, ct_kevent_t *e, timespec_t *tsp)
+cte_publish(ct_equeue_t *q, ct_kevent_t *e, timespec_t *tsp, boolean_t mayexist)
 {
 	ASSERT(MUTEX_HELD(&q->ctq_lock));
 
 	q->ctq_atime = *tsp;
 
 	/*
+	 * If this event may already exist on this queue, check to see if it
+	 * is already there and return if so.
+	 */
+	if (mayexist && list_link_active((list_node_t *)((uintptr_t)e +
+	    q->ctq_events.list_offset))) {
+		mutex_exit(&q->ctq_lock);
+		cte_rele(e);
+		return;
+	}
+
+	/*
 	 * Don't publish if the event is informative and there aren't
 	 * any listeners, or if the queue has been shut down.
 	 */
@@ -2247,6 +2266,8 @@
 	/*
 	 * Enqueue event
 	 */
+	VERIFY(!list_link_active((list_node_t *)
+	    ((uintptr_t)e + q->ctq_events.list_offset)));
 	list_insert_tail(&q->ctq_events, e);
 
 	/*
@@ -2318,14 +2339,14 @@
 			ct->ct_evcnt++;
 	}
 	mutex_exit(&ct->ct_lock);
-	cte_publish(&ct->ct_events, e, &ts);
+	cte_publish(&ct->ct_events, e, &ts, B_FALSE);
 
 	/*
 	 * CTEL_BUNDLE - Next deliver to the contract type's bundle
 	 * queue.
 	 */
 	mutex_enter(&ct->ct_type->ct_type_events.ctq_lock);
-	cte_publish(&ct->ct_type->ct_type_events, e, &ts);
+	cte_publish(&ct->ct_type->ct_type_events, e, &ts, B_FALSE);
 
 	/*
 	 * CTEL_PBUNDLE - Finally, if the contract has an owner,
@@ -2342,7 +2363,14 @@
 		q = ct->ct_owner->p_ct_equeue[ct->ct_type->ct_type_index];
 		mutex_enter(&q->ctq_lock);
 		mutex_exit(&ct->ct_lock);
-		cte_publish(q, e, &ts);
+
+		/*
+		 * It is possible for this code to race with adoption; we
+		 * publish the event indicating that the event may already
+		 * be enqueued because adoption beat us to it (in which case
+		 * cte_pubish() does nothing).
+		 */
+		cte_publish(q, e, &ts, B_TRUE);
 	} else {
 		mutex_exit(&ct->ct_lock);
 		cte_rele(e);