[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Pre-approval for cwidget upload to fix a deadlock (511708 and friends)



  Hi, release team,

  I've finally tracked down a nasty deadlock in the cwidget code
(#511708 and friends).  The symptoms are that under certain conditions,
which include but may not be limited to running an etch kernel and being
afflicted by the Curse of the Devil Bunny [0], the curses interface to
aptitude will hang up after running dpkg.  See the bug log for the gory
details of what was going on.

  I would really like to get this fix into lenny, if possible.  If I
prepare an upload of libcwidget3&friends, will it be allowed in?

    Thanks,
  Daniel

  [0] ok, I made that one up.
diff --git a/src/cwidget/toplevel.cc b/src/cwidget/toplevel.cc
index 7cfd048..78fa555 100644
--- a/src/cwidget/toplevel.cc
+++ b/src/cwidget/toplevel.cc
@@ -446,8 +446,13 @@ namespace cwidget
 
       void operator()()
       {
-	threads::mutex::lock l(input_event_mutex);
-	input_event_fired = false;
+	// Don't hold the lock for longer than we need to (we lock it
+	// here to be extra-paranoid about changing the value of
+	// input_event_fired).
+	{
+	  threads::mutex::lock l(input_event_mutex);
+	  input_event_fired = false;
+	}
 
 	// Important note: this routine only blocks indefinitely in
 	// pthread_cond_wait(), assuming no bugs in post_event().
@@ -485,6 +490,12 @@ namespace cwidget
 	      }
 	    else
 	      {
+		// Lock the mutex and wait on the condition variable.
+		// We have to be careful to release the mutex when we
+		// leave this scope; otherwise we could end up being
+		// canceled while we hold the mutex, which leads to
+		// horrible stuff like bug #511708.
+		threads::mutex::lock l(input_event_mutex);
 		post_event(new get_input_event(input_event_mutex,
 					       input_event_fired,
 					       input_event_condition));

Reply to: