Changeset db6f06a for src

Timestamp:

Feb 13, 2017, 5:04:43 PM (7 years ago)

Author:

Thierry Delisle <tdelisle@…>

Branches:

ADT, aaron-thesis, arm-eh, ast-experimental, cleanup-dtors, deferred_resn, demangler, enum, forall-pointer-decay, jacob/cs343-translation, jenkins-sandbox, master, new-ast, new-ast-unique-expr, new-env, no_list, persistent-indexer, pthread-emulation, qualifiedEnum, resolv-new, with_gc

Children:

ee897e4b

Parents:

75f3522

Message:

Implemented better condition lock to solve race condition on thread/processor termination

Location:

src/libcfa/concurrency

Files:

• coroutines.c (modified) (3 diffs)
• invoke.h (modified) (3 diffs)
• kernel (modified) (4 diffs)
• kernel.c (modified) (12 diffs)
• kernel_private.h (modified) (2 diffs)
• threads.c (modified) (3 diffs)

src/libcfa/concurrency/coroutines.c

@@ -61 +61 @@
 
 void ?{}(coroutine* this) {
-        this->name = "Anonymous Coroutine";
-        this->errno_ = 0;
-        this->state = Start;
-      this->notHalted = true;
-        this->starter = NULL;
-        this->last = NULL;
+        this{ "Anonymous Coroutine" };
 }
 
@@ -73 +68 @@
         this->errno_ = 0;
         this->state = Start;
-      this->notHalted = true;
+        this->notHalted = true;
         this->starter = NULL;
         this->last = NULL;
@@ -169 +164 @@
         this->context = this->base;
         this->top = (char *)this->context + cxtSize;
-
-        LIB_DEBUG_PRINTF("Coroutine : created stack %p\n", this->base);
 }
 

src/libcfa/concurrency/invoke.h

@@ -30 +30 @@
       #define SCHEDULER_CAPACITY 10
 
+      struct spinlock {
+            volatile int lock;
+      };
+
       struct simple_thread_list {
             struct thread * head;
             struct thread ** tail;
+      };
+
+      // struct simple_lock {
+      //        struct simple_thread_list blocked;
+      // };
+
+      struct signal_once {
+            volatile bool condition;
+            struct spinlock lock;
+            struct simple_thread_list blocked;
       };
 
@@ -40 +54 @@
             void append( struct simple_thread_list *, struct thread * );
             struct thread * pop_head( struct simple_thread_list * );
+
+            // void ?{}(simple_lock * this);
+            // void ^?{}(simple_lock * this);
+
+            void ?{}(spinlock * this);
+            void ^?{}(spinlock * this);
+
+            void ?{}(signal_once * this);
+            void ^?{}(signal_once * this);
       }
       #endif
@@ -60 +83 @@
             int errno_;                         // copy of global UNIX variable errno
             enum coroutine_state state; // current execution status for coroutine
-            bool notHalted;                     // indicate if execuation state is not halted
-
+            bool notHalted;                   // indicate if execuation state is not halted
             struct coroutine *starter;  // first coroutine to resume this one
             struct coroutine *last;             // last coroutine to resume this one
       };
 
-      struct simple_lock {
-        struct simple_thread_list blocked;
-      };
-
       struct thread {
             struct coroutine c;
-            struct simple_lock lock;
+            signal_once terminated;             // indicate if execuation state is not halted
             struct thread * next;
       };

src/libcfa/concurrency/kernel

@@ -27 +27 @@
 
 //-----------------------------------------------------------------------------
+// Locks
+// void lock( simple_lock * );
+// void lock( simple_lock *, spinlock * );
+// void unlock( simple_lock * );
+
+void lock( spinlock * );
+void unlock( spinlock * );
+
+void wait( signal_once * );
+void signal( signal_once * );
+
+//-----------------------------------------------------------------------------
 // Cluster
 struct cluster {
         simple_thread_list ready_queue;
-        // pthread_spinlock_t lock;
+        spinlock lock;
 };
 
@@ -38 +50 @@
 //-----------------------------------------------------------------------------
 // Processor
-enum ProcessorAction { 
-        Reschedule,
-        NoAction
+enum FinishOpCode { No_Action, Release, Schedule, Release_Schedule };
+struct FinishAction {
+        FinishOpCode action_code;
+        thread * thrd;
+        spinlock * lock;
 };
+static inline void ?{}(FinishAction * this) { 
+        this->action_code = No_Action;
+        this->thrd = NULL;
+        this->lock = NULL;
+}
+static inline void ^?{}(FinishAction * this) {}
 
 struct processor {
@@ -49 +69 @@
         thread * current_thread;
         pthread_t kernel_thread;
-        simple_lock lock;
-        volatile bool terminated;
-        ProcessorAction thread_action;
+        
+        signal_once terminated;
+        volatile bool is_terminated;
+
+        struct FinishAction finish;
 };
 
@@ -57 +79 @@
 void ?{}(processor * this, cluster * cltr);
 void ^?{}(processor * this);
-
-//-----------------------------------------------------------------------------
-// Locks
-
-void ?{}(simple_lock * this);
-void ^?{}(simple_lock * this);
-
-void lock( simple_lock * );
-void unlock( simple_lock * );
 
 #endif //KERNEL_H

src/libcfa/concurrency/kernel.c

@@ -141 +141 @@
 }
 
-void start(processor * this);
-
 void ?{}(processor * this) {
         this{ systemCluster };
@@ -151 +149 @@
         this->current_coroutine = NULL;
         this->current_thread = NULL;
-        (&this->lock){};
-        this->terminated = false;
+        (&this->terminated){};
+        this->is_terminated = false;
 
         start( this );
@@ -161 +159 @@
         this->current_coroutine = NULL;
         this->current_thread = NULL;
-        (&this->lock){};
-        this->terminated = false;
+        (&this->terminated){};
+        this->is_terminated = false;
 
         this->runner = runner;
@@ -170 +168 @@
 
 void ^?{}(processor * this) {
-        if( ! this->terminated ) {
+        if( ! this->is_terminated ) {
                 LIB_DEBUG_PRINTF("Kernel : core %p signaling termination\n", this);
-                this->terminated = true;
-                lock( &this->lock );
+                this->is_terminated = true;
+                wait( &this->terminated );
         }
 }
@@ -194 +192 @@
         LIB_DEBUG_PRINTF("Kernel : core %p starting\n", this);
 
-        fenv_t envp;
-        fegetenv( &envp );
-        LIB_DEBUG_PRINTF("Kernel : mxcsr %x\n", envp.__mxcsr);
-
         thread * readyThread = NULL;
-        for( unsigned int spin_count = 0; ! this->terminated; spin_count++ ) 
+        for( unsigned int spin_count = 0; ! this->is_terminated; spin_count++ ) 
         {
                 readyThread = nextThread( this->cltr );
@@ -208 +202 @@
 
                         //Some actions need to be taken from the kernel
-                        finishRunning(this, readyThread);
+                        finishRunning(this);
 
                         spin_count = 0;
@@ -219 +213 @@
 
         LIB_DEBUG_PRINTF("Kernel : core %p unlocking thread\n", this);
-        unlock( &this->lock );
+        signal( &this->terminated );
         LIB_DEBUG_PRINTF("Kernel : core %p terminated\n", this);
 }
@@ -230 +224 @@
         
         //Reset the terminating actions here
-        this->thread_action = NoAction;
+        this->finish.action_code = No_Action;
 
         //Update global state
@@ -242 +236 @@
 // Once a thread has finished running, some of 
 // its final actions must be executed from the kernel
-void finishRunning(processor * this, thread * thrd) {
-        if(this->thread_action == Reschedule) {
-                ScheduleThread( thrd );
+void finishRunning(processor * this) {
+        if( this->finish.action_code == Release ) {
+                unlock( this->finish.lock );
+        }
+        else if( this->finish.action_code == Schedule ) {
+                ScheduleThread( this->finish.thrd );
+        }
+        else if( this->finish.action_code == Release_Schedule ) {
+                unlock( this->finish.lock );            
+                ScheduleThread( this->finish.thrd );
+        }
+        else {
+                assert(this->finish.action_code == No_Action);
         }
 }
@@ -310 +314 @@
         assertf( thrd->next == NULL, "Expected null got %p", thrd->next );
         
-        spin_lock( &lock );
+        lock( &systemProcessor->cltr->lock );
         append( &systemProcessor->cltr->ready_queue, thrd );
-        spin_unlock( &lock );
+        unlock( &systemProcessor->cltr->lock );
+}
+
+thread * nextThread(cluster * this) {
+        lock( &this->lock );
+        thread * head = pop_head( &this->ready_queue );
+        unlock( &this->lock );
+        return head;
 }
 
 void ScheduleInternal() {
-        get_this_processor()->thread_action = Reschedule;
         suspend();
 }
 
-thread * nextThread(cluster * this) {
-        spin_lock( &lock );
-        thread * head = pop_head( &this->ready_queue );
-        spin_unlock( &lock );
-        return head;
+void ScheduleInternal( spinlock * lock ) {
+        get_this_processor()->finish.action_code = Release;
+        get_this_processor()->finish.lock = lock;
+        suspend();
+}
+
+void ScheduleInternal( thread * thrd ) {
+        get_this_processor()->finish.action_code = Schedule;
+        get_this_processor()->finish.thrd = thrd;
+        suspend();
+}
+
+void ScheduleInternal( spinlock * lock, thread * thrd ) {
+        get_this_processor()->finish.action_code = Release_Schedule;
+        get_this_processor()->finish.lock = lock;
+        get_this_processor()->finish.thrd = thrd;
+        suspend();
 }
 
@@ -374 +396 @@
         // When its coroutine terminates, it return control to the mainThread
         // which is currently here
-        systemProcessor->terminated = true;
+        systemProcessor->is_terminated = true;
         suspend();
 
@@ -393 +415 @@
 //-----------------------------------------------------------------------------
 // Locks
-void ?{}( simple_lock * this ) {
-        ( &this->blocked ){};
-}
-
-void ^?{}( simple_lock * this ) {
-
-}
-
-void lock( simple_lock * this ) {
+// void ?{}( simple_lock * this ) {
+//      ( &this->blocked ){};
+// }
+
+// void ^?{}( simple_lock * this ) {
+
+// }
+
+// void lock( simple_lock * this ) {
+//      {
+//              spin_lock( &lock );
+//              append( &this->blocked, this_thread() );
+//              spin_unlock( &lock );
+//      }
+//      ScheduleInternal();
+// }
+
+// void lock( simple_lock * this, spinlock * to_release ) {
+//      {
+//              spin_lock( &lock );
+//              append( &this->blocked, this_thread() );
+//              spin_unlock( &lock );
+//      }
+//      ScheduleInternal( to_release );
+//      lock( to_release );
+// }
+
+// void unlock( simple_lock * this ) {
+//      thread * it;
+//      while( it = pop_head( &this->blocked) ) {
+//              ScheduleThread( it );
+//      }
+// }
+
+void ?{}( spinlock * this ) {
+        this->lock = 0;
+}
+void ^?{}( spinlock * this ) {
+
+}
+
+void lock( spinlock * this ) {
+        for ( unsigned int i = 1;; i += 1 ) {
+                if ( this->lock == 0 && __sync_lock_test_and_set_4( &this->lock, 1 ) == 0 ) break;
+        }
+}
+
+void unlock( spinlock * this ) {
+        __sync_lock_release_4( &this->lock );
+}
+
+void ?{}( signal_once * this ) {
+        this->condition = false;
+}
+void ^?{}( signal_once * this ) {
+
+}
+
+void wait( signal_once * this ) {
+        lock( &this->lock );
+        if( !this->condition ) {
+                append( &this->blocked, this_thread() );
+                ScheduleInternal( &this->lock );
+                lock( &this->lock );
+        }
+        unlock( &this->lock );
+}
+
+void signal( signal_once * this ) {
+        lock( &this->lock );
         {
-                spin_lock( &lock );
-                append( &this->blocked, this_thread() );
-                spin_unlock( &lock );
-        }
-        suspend();
-}
-
-void unlock( simple_lock * this ) {
-        thread * it;
-        while( it = pop_head( &this->blocked) ) {
-                ScheduleThread( it );
-        }
+                this->condition = true;
+
+                thread * it;
+                while( it = pop_head( &this->blocked) ) {
+                        ScheduleThread( it );
+                }
+        }
+        unlock( &this->lock );
 }
 

src/libcfa/concurrency/kernel_private.h

@@ -24 +24 @@
 // Scheduler
 void ScheduleThread( thread * );
+thread * nextThread(cluster * this);
+
 void ScheduleInternal();
-thread * nextThread(cluster * this);
+void ScheduleInternal(spinlock * lock);
+void ScheduleInternal(thread * thrd);
+void ScheduleInternal(spinlock * lock, thread * thrd);
 
 //-----------------------------------------------------------------------------
@@ -37 +41 @@
 
 void main(processorCtx_t *);
+void start(processor * this);
 void runThread(processor * this, thread * dst);
-void finishRunning(processor * this, thread * thrd);
+void finishRunning(processor * this);
 void spin(processor * this, unsigned int * spin_count);
 

src/libcfa/concurrency/threads.c

@@ -44 +44 @@
         (&this->c){};
         this->c.name = "Anonymous Coroutine";
-        (&this->lock){};
+        (&this->terminated){};
         this->next = NULL;
 }
@@ -90 +90 @@
 forall( dtype T | is_thread(T) )
 void stop( T* this ) {
-        thread*  thrd = get_thread(this);
-        if( thrd->c.notHalted ) {
-                lock( &thrd->lock );
-        }
+        wait( & get_thread(this)->terminated ); 
 }
 
 void yield( void ) {
-        ScheduleInternal();
+        ScheduleInternal( get_this_processor()->current_thread );
 }
 
@@ -124 +121 @@
         void __thread_signal_termination( thread * this ) {
                 this->c.state = Halt;
-                this->c.notHalted = false;
-                unlock( &this->lock );
+                LIB_DEBUG_PRINTF("Thread end : %p\n", this);
+                signal( &this->terminated );    
         }
 }