When locking a mutex, ensure the task has a context before attempting to lock.

src/thread/mutex.cpp

@@ -23,19 +23,22 @@ namespace fc {
   }
 
   /**
-   *  @param  next - is set to the next context to get the lock.
+   *  @param  last_context - is set to the next context to get the lock (the next-to-last element of the list)
    *  @return the last context (the one with the lock)
    */
-  static fc::context* get_tail( fc::context* h, fc::context*& next ) {
-    next = 0;
-    fc::context* n = h;
-    if( !n ) return n;
-    while( n->next_blocked_mutex ) { 
-      next = n;
-      n=n->next_blocked_mutex;
+  static fc::context* get_tail( fc::context* list_head, fc::context*& context_to_unblock ) {
+    context_to_unblock = 0;
+    fc::context* list_context_iter = list_head;
+    if( !list_context_iter ) 
+      return list_context_iter;
+    while( list_context_iter->next_blocked_mutex ) 
+    { 
+      context_to_unblock = list_context_iter;
+      list_context_iter = list_context_iter->next_blocked_mutex;
     }
-    return n;
+    return list_context_iter;
   }
+
   static fc::context* remove( fc::context* head, fc::context* target ) {
     fc::context* c = head;
     fc::context* p = 0;
@@ -114,17 +117,21 @@ namespace fc {
   }
 
   void mutex::lock() {
-    fc::context* n  = 0;
-    fc::context* cc = fc::thread::current().my->current;
+    fc::context* current_context = fc::thread::current().my->current;
+    if( !current_context ) 
+      current_context = fc::thread::current().my->current = new fc::context( &fc::thread::current() );
+
     {
       fc::unique_lock<fc::spin_yield_lock> lock(m_blist_lock);
       if( !m_blist ) { 
-        m_blist = cc;
+        m_blist = current_context;
+        assert(!current_context->next_blocked_mutex);
         return;
       }
 
       // allow recusive locks
-      if ( get_tail( m_blist, n ) == cc ) {
+      fc::context* dummy_context_to_unblock  = 0;
+      if ( get_tail( m_blist, dummy_context_to_unblock ) == current_context ) {
         assert(false);
         // EMF: I think recursive locks are currently broken -- we need to 
 	// keep track of how many times this mutex has been locked by the
@@ -132,9 +139,10 @@ namespace fc {
 	// the next context only if the count drops to zero
         return;
       }
-      cc->next_blocked_mutex = m_blist;
-      m_blist = cc;
+      current_context->next_blocked_mutex = m_blist;
+      m_blist = current_context;
 
+#if 0
       int cnt = 0;
       auto i = m_blist;
       while( i ) {
@@ -142,25 +150,26 @@ namespace fc {
         ++cnt;
       }
       //wlog( "wait queue len %1%", cnt );
+#endif
     }
 
     try {
       fc::thread::current().yield(false);
-      BOOST_ASSERT( cc->next_blocked_mutex == 0 );
+      BOOST_ASSERT( current_context->next_blocked_mutex == 0 );
     } catch ( ... ) {
       wlog( "lock threw" );
-      cleanup( *this, m_blist_lock, m_blist, cc);
+      cleanup( *this, m_blist_lock, m_blist, current_context);
       throw;
     }
   }
 
   void mutex::unlock() {
-    fc::context* next = 0;
+    fc::context* context_to_unblock = 0;
     { fc::unique_lock<fc::spin_yield_lock> lock(m_blist_lock);
-      get_tail(m_blist, next);
-      if( next ) {
-        next->next_blocked_mutex = 0;
-        next->ctx_thread->my->unblock( next );
+      get_tail(m_blist, context_to_unblock);
+      if( context_to_unblock ) {
+        context_to_unblock->next_blocked_mutex = 0;
+        context_to_unblock->ctx_thread->my->unblock( context_to_unblock );
       } else {
         m_blist   = 0;
       }

tests/task_cancel.cpp

@@ -12,13 +12,54 @@ BOOST_AUTO_TEST_CASE( leave_mutex_locked )
 {
   {
     fc::mutex test_mutex;
-    fc::future<void> test_task = fc::async([&](){ fc::scoped_lock<fc::mutex> test_lock(test_mutex); for (int i = 0; i < 10; ++i) fc::usleep(fc::seconds(1));});
+    fc::future<void> test_task = fc::async([&](){ 
+      fc::scoped_lock<fc::mutex> test_lock(test_mutex); 
+      for (int i = 0; i < 10; ++i) 
+        fc::usleep(fc::seconds(1));
+    }, "test_task");
     fc::usleep(fc::seconds(3));
     test_task.cancel_and_wait();
   }
   BOOST_TEST_PASSPOINT();
 }
 
+BOOST_AUTO_TEST_CASE( cancel_task_blocked_on_mutex)
+{
+  {
+    fc::mutex test_mutex;
+    fc::future<void> test_task;
+    {
+      fc::scoped_lock<fc::mutex> test_lock(test_mutex); 
+      test_task = fc::async([&test_mutex](){ 
+        BOOST_TEST_MESSAGE("--- In test_task, locking mutex");
+        fc::scoped_lock<fc::mutex> async_task_test_lock(test_mutex); 
+        BOOST_TEST_MESSAGE("--- In test_task, mutex locked, commencing sleep");
+        for (int i = 0; i < 10; ++i) 
+          fc::usleep(fc::seconds(1));
+        BOOST_TEST_MESSAGE("--- In test_task, sleeps done, exiting");
+      }, "test_task");
+      fc::usleep(fc::seconds(3));
+      test_task.cancel();
+      try
+      {
+        test_task.wait(fc::seconds(1));
+        BOOST_ERROR("test should have been canceled, not exited cleanly");
+      }
+      catch (const fc::canceled_exception&)
+      {
+        BOOST_TEST_PASSPOINT();
+      }
+      catch (const fc::timeout_exception&)
+      {
+        BOOST_ERROR("unable to cancel task blocked on mutex");
+      }
+      BOOST_TEST_MESSAGE("Unlocking mutex locked from the main task so the test task will have the opportunity to lock it and be canceled");
+    }
+    test_task.cancel_and_wait();
+  }
+}
+
+
 BOOST_AUTO_TEST_CASE( test_non_preemptable_assertion )
 {
   return; // this isn't a real test, because the thing it tries to test works by asserting, not by throwing