Changeset 5afb49a for libcfa/src/concurrency

Timestamp:

Oct 16, 2020, 3:31:02 PM (4 years ago)

Author:

Thierry Delisle <tdelisle@…>

Branches:

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

Children:

8da7421f

Parents:

d21dd3cb

Message:

Split thread_leave so backend is called from the kernel once the kernel no longer needs the thread.
This hopefully solves the non-deterministic crashes in the build.

Location:

libcfa/src/concurrency

Files:

• kernel.cfa (modified) (3 diffs)
• kernel.hfa (modified) (1 diff)
• kernel/startup.cfa (modified) (1 diff)
• kernel_private.hfa (modified) (1 diff)
• monitor.cfa (modified) (1 diff)

libcfa/src/concurrency/kernel.cfa

@@ -252 +252 @@
                 /* paranoid */ verify( kernelTLS.this_thread == thrd_dst );
                 /* paranoid */ verify( thrd_dst->context.SP );
+                /* paranoid */ verify( thrd_dst->state != Halted );
                 /* paranoid */ verifyf( ((uintptr_t)thrd_dst->context.SP) < ((uintptr_t)__get_stack(thrd_dst->curr_cor)->base ) || thrd_dst->curr_cor == proc_cor, "ERROR : Destination $thread %p has been corrupted.\n StackPointer too small.\n", thrd_dst ); // add escape condition if we are setting up the processor
                 /* paranoid */ verifyf( ((uintptr_t)thrd_dst->context.SP) > ((uintptr_t)__get_stack(thrd_dst->curr_cor)->limit) || thrd_dst->curr_cor == proc_cor, "ERROR : Destination $thread %p has been corrupted.\n StackPointer too large.\n", thrd_dst ); // add escape condition if we are setting up the processor
@@ -287 +288 @@
                 if(unlikely(thrd_dst->state == Halted)) {
                         // The thread has halted, it should never be scheduled/run again
-                        // We may need to wake someone up here since
-                        unpark( this->destroyer );
-                        this->destroyer = 0p;
+                        // finish the thread
+                        __thread_finish( thrd_dst );
                         break RUNNING;
                 }
@@ -448 +448 @@
 }
 
-// KERNEL ONLY
-void __leave_thread() {
-        /* paranoid */ verify( ! kernelTLS.preemption_state.enabled );
-        returnToKernel();
-        abort();
+extern "C" {
+        // Leave the thread monitor
+        // last routine called by a thread.
+        // Should never return
+        void __cfactx_thrd_leave() {
+                $thread * thrd = TL_GET( this_thread );
+                $monitor * this = &thrd->self_mon;
+
+                // Lock the monitor now
+                lock( this->lock __cfaabi_dbg_ctx2 );
+
+                disable_interrupts();
+
+                thrd->state = Halted;
+
+                if( thrd != this->owner || this->recursion != 1) { abort( "Thread internal monitor has unbalanced recursion" ); }
+
+                // Leave the thread
+                /* paranoid */ verify( ! kernelTLS.preemption_state.enabled );
+                returnToKernel();
+                abort();
+
+                // Control flow should never reach here!
+        }
 }
 

libcfa/src/concurrency/kernel.hfa

@@ -79 +79 @@
         // Handle to pthreads
         pthread_t kernel_thread;
-
-        // RunThread data
-        // Action to do after a thread is ran
-        $thread * destroyer;
 
         // Preemption data

libcfa/src/concurrency/kernel/startup.cfa

@@ -474 +474 @@
         this.cltr = &_cltr;
         full_proc = true;
-        destroyer = 0p;
         do_terminate = false;
         preemption_alarm = 0p;

libcfa/src/concurrency/kernel_private.hfa

@@ -39 +39 @@
 ;
 
-//Block current thread and release/wake-up the following resources
-void __leave_thread() __attribute__((noreturn));
+//release/wake-up the following resources
+void __thread_finish( $thread * thrd );
 
 //-----------------------------------------------------------------------------

libcfa/src/concurrency/monitor.cfa

@@ -281 +281 @@
 }
 
-extern "C" {
-        // Leave the thread monitor
-        // last routine called by a thread.
-        // Should never return
-        void __cfactx_thrd_leave() {
-                $thread * thrd = TL_GET( this_thread );
-                $monitor * this = &thrd->self_mon;
-
-                // Lock the monitor now
-                lock( this->lock __cfaabi_dbg_ctx2 );
-
-                disable_interrupts();
-
-                thrd->state = Halted;
-
-                /* paranoid */ verifyf( thrd == this->owner, "Expected owner to be %p, got %p (r: %i, m: %p)", thrd, this->owner, this->recursion, this );
-
-                // Leaving a recursion level, decrement the counter
-                this->recursion -= 1;
-
-                // If we haven't left the last level of recursion
-                // it must mean there is an error
-                if( this->recursion != 0) { abort( "Thread internal monitor has unbalanced recursion" ); }
-
-                // Fetch the next thread, can be null
-                $thread * new_owner = next_thread( this );
-
-                // Release the monitor lock
-                unlock( this->lock );
-
-                // Unpark the next owner if needed
-                /* paranoid */ verifyf( !new_owner || new_owner == this->owner, "Expected owner to be %p, got %p (m: %p)", new_owner, this->owner, this );
-                /* paranoid */ verify( ! kernelTLS.preemption_state.enabled );
-                /* paranoid */ verify( ! kernelTLS.this_processor->destroyer );
-                /* paranoid */ verify( thrd->state == Halted );
-
-                kernelTLS.this_processor->destroyer = new_owner;
-
-                // Leave the thread
-                __leave_thread();
-
-                // Control flow should never reach here!
-        }
+void __thread_finish( $thread * thrd ) {
+        $monitor * this = &thrd->self_mon;
+
+        // Lock the monitor now
+        /* paranoid */ verify( this->lock.lock );
+        /* paranoid */ verifyf( thrd == this->owner, "Expected owner to be %p, got %p (r: %i, m: %p)", thrd, this->owner, this->recursion, this );
+        /* paranoid */ verify( ! kernelTLS.preemption_state.enabled );
+        /* paranoid */ verify( thrd->state == Halted );
+        /* paranoid */ verify( this->recursion == 1 );
+
+        // Leaving a recursion level, decrement the counter
+        this->recursion -= 1;
+        this->owner = 0p;
+
+        // Fetch the next thread, can be null
+        $thread * new_owner = next_thread( this );
+
+        // Release the monitor lock
+        unlock( this->lock );
+
+        // Unpark the next owner if needed
+        /* paranoid */ verifyf( !new_owner || new_owner == this->owner, "Expected owner to be %p, got %p (m: %p)", new_owner, this->owner, this );
+        /* paranoid */ verify( ! kernelTLS.preemption_state.enabled );
+        /* paranoid */ verify( thrd->state == Halted );
+        unpark( new_owner );
 }