Changeset 3ea8ad1

Timestamp:

Nov 18, 2020, 2:28:02 PM (3 years ago)

Author:

Thierry Delisle <tdelisle@…>

Branches:

ADT, arm-eh, enum, forall-pointer-decay, jacob/cs343-translation, master, new-ast-unique-expr, pthread-emulation, qualifiedEnum

Children:

69d1748

Parents:

9d6e1b8a

Message:

Added more checks for thread termination synchronization

Location:

libcfa/src/concurrency

Files:

• invoke.h (modified) (1 diff)
• kernel.cfa (modified) (2 diffs)
• monitor.cfa (modified) (7 diffs)
• thread.cfa (modified) (1 diff)

libcfa/src/concurrency/invoke.h

@@ -68 +68 @@
         };
 
-        enum __Coroutine_State { Halted, Start, Primed, Blocked, Ready, Active, Cancelled };
+        enum __Coroutine_State { Halted, Start, Primed, Blocked, Ready, Active, Cancelled, Halting };
 
         struct $coroutine {

libcfa/src/concurrency/kernel.cfa

@@ -288 +288 @@
                 }
 
-                if(unlikely(thrd_dst->state == Halted)) {
+                if(unlikely(thrd_dst->state == Halting)) {
                         // The thread has halted, it should never be scheduled/run again
                         // finish the thread
@@ -484 +484 @@
                 /* paranoid */ verifyf( ((uintptr_t)thrd->context.SP) < ((uintptr_t)__get_stack(thrd->curr_cor)->base ), "ERROR : $thread %p has been corrupted.\n StackPointer too small.\n", thrd );
 
-                thrd->state = Halted;
+                thrd->state = Halting;
                 if( TICKET_RUNNING != thrd->ticket ) { abort( "Thread terminated with pending unpark" ); }
                 if( thrd != this->owner ) { abort( "Thread internal monitor has incorrect owner" ); }

libcfa/src/concurrency/monitor.cfa

@@ -142 +142 @@
 static void __dtor_enter( $monitor * this, fptr_t func, bool join ) {
         $thread * thrd = active_thread();
+        #if defined( __CFA_WITH_VERIFY__ )
+                bool is_thrd = this == &thrd->self_mon;
+        #endif
 
         // Lock the monitor spinlock
@@ -155 +158 @@
                 __set_owner( this, thrd );
 
-                verifyf( active_thread() == this->owner, "Expected owner to be %p, got %p (r: %i, m: %p)", active_thread(), this->owner, this->recursion, this );
+                /* paranoid */ verifyf( active_thread() == this->owner, "Expected owner to be %p, got %p (r: %i, m: %p)", active_thread(), this->owner, this->recursion, this );
+                /* paranoid */ verify( !is_thrd || thrd->state == Halted || thrd->state == Cancelled );
 
                 unlock( this->lock );
@@ -175 +179 @@
                 __set_owner( this, thrd );
 
-                verifyf( active_thread() == this->owner, "Expected owner to be %p, got %p (r: %i, m: %p)", active_thread(), this->owner, this->recursion, this );
+                /* paranoid */ verifyf( active_thread() == this->owner, "Expected owner to be %p, got %p (r: %i, m: %p)", active_thread(), this->owner, this->recursion, this );
+                /* paranoid */ verify( !is_thrd || thrd->state == Halted || thrd->state == Cancelled );
 
                 unlock( this->lock );
                 return;
         }
+
+        // The monitor is busy, if this is a thread and the thread owns itself, it better be active
+        /* paranoid */ verify( !is_thrd || this->owner != thrd || (thrd->state != Halted && thrd->state != Cancelled) );
 
         __lock_size_t count = 1;
@@ -209 +217 @@
                 // Some one was waiting for us, enter
                 /* paranoid */ verifyf( active_thread() == this->owner, "Expected owner to be %p, got %p (r: %i, m: %p)", active_thread(), this->owner, this->recursion, this );
+
+                __cfaabi_dbg_print_safe( "Kernel : Destroying %p\n", this);
+                return;
         }
         else {
@@ -228 +239 @@
                 return;
         }
-
-        __cfaabi_dbg_print_safe( "Kernel : Destroying %p\n", this);
-
 }
 
@@ -294 +302 @@
 
         /* paranoid */ verifyf( thrd == this->owner, "Expected owner to be %p, got %p (r: %i, m: %p)", thrd, this->owner, this->recursion, this );
-        /* paranoid */ verify( thrd->state == Halted );
+        /* paranoid */ verify( thrd->state == Halting );
         /* paranoid */ verify( this->recursion == 1 );
 
@@ -303 +311 @@
         // Fetch the next thread, can be null
         $thread * new_owner = next_thread( this );
+
+        // Mark the state as fully halted
+        thrd->state = Halted;
 
         // Release the monitor lock

libcfa/src/concurrency/thread.cfa

@@ -82 +82 @@
                 T & thrd, void(*defaultResumptionHandler)(ThreadCancelled(T) &)) {
         $monitor * m = get_monitor(thrd);
+        $thread * desc = get_thread(thrd);
+
+        // Setup the monitor guard
         void (*dtor)(T& mutex this) = ^?{};
         bool join = defaultResumptionHandler != (void(*)(ThreadCancelled(T)&))0;
         (this.mg){&m, (void(*)())dtor, join};
 
+
+        /* paranoid */ verifyf( Halted == desc->state || Cancelled == desc->state, "Expected thread to be Halted or Cancelled, was %d\n", (int)desc->state );
+
         // After the guard set-up and any wait, check for cancellation.
-        $thread * desc = get_thread(thrd);
         struct _Unwind_Exception * cancellation = desc->self_cor.cancellation;
         if ( likely( 0p == cancellation ) ) {