Fix problem of deemon (appearing to) brieflyx hang during exit

Author: GrieferAtWork

include/deemon/thread.h

@@ -362,6 +362,12 @@ typedef struct Dee_thread_object {
 #define DeeThread_WasInterrupted(self) (__hybrid_atomic_load(&(self)->t_state, __ATOMIC_ACQUIRE) & Dee_THREAD_STATE_INTERRUPTED)
 #define DeeThread_WasDetached(self)    (!(__hybrid_atomic_load(&(self)->t_state, __ATOMIC_ACQUIRE) & Dee_THREAD_STATE_HASTHREAD))
 #define DeeThread_HasCrashed(self)     ((DeeThread_HasTerminated(self) || (self) == DeeThread_Self()) && (self)->t_exceptsz > 0)
+/* Check if there's work to-be done that can be handled by `DeeThread_CheckInterruptNoInt()' */
+#define DeeThread_WasInterruptedNoInt(self)                       \
+	((__hybrid_atomic_load(&(self)->t_state, __ATOMIC_ACQUIRE) &  \
+	  (Dee_THREAD_STATE_SUSPENDING | Dee_THREAD_STATE_SUSPENDED | \
+	   Dee_THREAD_STATE_INTERRUPTED)) ==                          \
+	 (Dee_THREAD_STATE_SUSPENDING | Dee_THREAD_STATE_INTERRUPTED))
 #endif /* !CONFIG_NO_THREADS */
 
 #define _DeeThread_AcquireInterrupt(self) _DeeThread__AcquireStateLock(self, Dee_THREAD_STATE_INTERRUPTING)
@@ -473,22 +479,37 @@ DeeThread_Trace(/*Thread*/ DeeObject *__restrict self);
  * only those that jump backwards in code (aka. is guarantied to be checked
  * periodically during execution of any kind of infinite-loop). */
 DFUNDEF WUNUSED int (DCALL DeeThread_CheckInterrupt)(void);
+/* Same as `DeeThread_CheckInterrupt()', but only handle interrupts that
+ * will not invoke user-code or throw errors. (e.g.: this can be used to
+ * handle thread suspend requests) */
+DFUNDEF void (DCALL DeeThread_CheckInterruptNoInt)(void);
 
 #ifdef CONFIG_BUILDING_DEEMON
 /* Same as `DeeThread_CheckInterrupt()', but faster
  * if the caller already knows their own thread object. */
 INTDEF WUNUSED NONNULL((1)) int
-(DCALL DeeThread_CheckInterruptSelf)(DeeThreadObject *__restrict thread_self);
+(DCALL DeeThread_CheckInterruptSelf)(DeeThreadObject *__restrict self);
+INTDEF NONNULL((1)) void
+(DCALL DeeThread_CheckInterruptNoIntSelf)(DeeThreadObject *__restrict self);
 #ifndef __OPTIMIZE_SIZE__
 /* Since `DeeThread_Self()' is marked as ATTR_CONST, in many cases where the calling function
  * already knows about its current thread from a previous call to `DeeThread_Self()', the compiler
  * will be able to optimize the secondary lookup away, making the code slightly faster. */
-#define DeeThread_CheckInterrupt() DeeThread_CheckInterruptSelf(DeeThread_Self())
+#define DeeThread_CheckInterrupt()      DeeThread_CheckInterruptSelf(DeeThread_Self())
+#define DeeThread_CheckInterruptNoInt() DeeThread_CheckInterruptNoIntSelf(DeeThread_Self())
 #endif /* !__OPTIMIZE_SIZE__ */
 #else /* CONFIG_BUILDING_DEEMON */
-#define DeeThread_CheckInterruptSelf(self) DeeThread_CheckInterrupt()
+#define DeeThread_CheckInterruptSelf(self)      DeeThread_CheckInterrupt()
+#define DeeThread_CheckInterruptNoIntSelf(self) DeeThread_CheckInterruptNoInt()
 #endif /* !CONFIG_BUILDING_DEEMON */
 
+#ifdef CONFIG_NO_THREADS
+#undef DeeThread_CheckInterruptNoInt
+#undef DeeThread_CheckInterruptNoIntSelf
+#define DeeThread_CheckInterruptNoInt()         (void)0
+#define DeeThread_CheckInterruptNoIntSelf(self) (void)0
+#endif /* CONFIG_NO_THREADS */
+
 /* Suspend/resume execution of the given thread.
  * WARNING: Do _NOT_ expose these functions to user-code.
  * WARNING: Do not attempt to suspend more than a single thread at once using this

src/deemon/runtime/gc-inspect.c.inl

@@ -220,7 +220,7 @@ GCSet_GetMarker(GCSet *self, DeeObject *ob) {
 /* @return: 1 : Already present
  * @return: 0 : Success, or already present
  * @return: -1: OOM failure (no error was thrown) */
-PRIVATE WUNUSED NONNULL((1, 2)) int DCALL
+PRIVATE NONNULL((1, 2)) int DCALL
 GCSet_SetMarker(GCSet *self, DeeObject *ob, bool marker) {
 	Dee_hash_t i, perturb, hash = DeeObject_HashGeneric(ob);
 	ASSERT((uintptr_t)marker == 0 || (uintptr_t)marker == 1);

src/deemon/runtime/gc.c

@@ -27,6 +27,7 @@
 #include <deemon/arg.h>                /* DeeArg_Unpack*, UNPxSIZ */
 #include <deemon/asm.h>                /* ASM_RET_NONE, instruction_t */
 #include <deemon/bool.h>               /* return_bool, return_false, return_true */
+#include <deemon/system.h>               /* return_bool, return_false, return_true */
 #include <deemon/class.h>              /* instance_clear, instance_tclear */
 #include <deemon/code.h>               /* DeeCodeObject, DeeCode_NAME, DeeCode_Type, DeeFunctionObject, DeeFunction_*, Dee_CODE_FFINALLY, instruction_t */
 #include <deemon/computed-operators.h> /* DEFIMPL, DEFIMPL_UNSUPPORTED */
@@ -73,6 +74,17 @@
 #define DBG_memset(dst, byte, n_bytes) (void)0
 #endif /* NDEBUG */
 
+#if !defined(NDEBUG) && 0
+#if 0
+#define GC_TRACE(...) fprintf(stderr, __VA_ARGS__)
+#else
+#define GC_TRACE(...) Dee_DPRINTF(__VA_ARGS__)
+#endif
+#else
+#define GC_TRACE(...) (void)0
+#define GC_TRACE_IS_NOOP
+#endif
+
 DECL_BEGIN
 
 #ifndef NDEBUG
@@ -211,10 +223,16 @@ PRIVATE Dee_atomic_lock_t gc_lock = Dee_ATOMIC_LOCK_INIT;
 #define gc_lock_available()  Dee_atomic_lock_available(&gc_lock)
 #define gc_lock_acquired()   Dee_atomic_lock_acquired(&gc_lock)
 #define gc_lock_tryacquire() Dee_atomic_lock_tryacquire(&gc_lock)
-#define gc_lock_acquire()    Dee_atomic_lock_acquire(&gc_lock)
 #define gc_lock_waitfor()    Dee_atomic_lock_waitfor(&gc_lock)
 #define _gc_lock_release()   Dee_atomic_lock_release(&gc_lock)
 
+PRIVATE void DCALL gc_lock_acquire(void) {
+	while (!gc_lock_tryacquire()) {
+		DeeThread_CheckInterruptNoInt();
+		SCHED_YIELD();
+	}
+}
+
 /* Called to (try to) collect GC objects after "gg_collect_on" of some
  * GC generation was reached. This function is must be called without
  * the calling thread holding any (internal) locks, include locks
@@ -445,11 +463,6 @@ PRIVATE void DCALL gc_lock_reap(unsigned int flags) {
 		gc_lock_reap_and_release(flags);
 }
 
-PRIVATE bool DCALL gc_lock_force_reap(unsigned int flags) {
-	gc_lock_acquire();
-	return gc_lock_reap_and_release(flags);
-}
-
 PRIVATE void DCALL gc_lock_acquire_and_reap(unsigned int flags) {
 	unsigned int status;
 again:
@@ -479,6 +492,14 @@ DeeGC_Track(DREF DeeObject *__restrict ob) {
 	bool must_collect;
 	struct Dee_gc_head *head = DeeGC_Head(ob);
 	ASSERT(!((uintptr_t)&head->gc_next & Dee_GC_FLAG_MASK));
+#ifndef GC_TRACE_IS_NOOP
+	if (atomic_read(&DeeThread_Self()->t_state) & Dee_THREAD_STATE_SHUTDOWNINTR) {
+		GC_TRACE("%llu: %p: DeeGC_Track(%s, %p)\n",
+		         DeeSystem_GetWalltime(), DeeThread_Self(),
+		         ob->ob_type->tp_name, ob);
+	}
+#endif /* !GC_TRACE_IS_NOOP */
+
 	if (!gc_lock_tryacquire()) {
 		/* Setup as a pending insert */
 		DeeObject *next;
@@ -788,19 +809,30 @@ PRIVATE WUNUSED bool DCALL
 gc_collect_acquire(bool allow_interrupts_and_errors) {
 	DeeObject *pending_remove;
 	DeeThreadObject *threads;
+	GC_TRACE("%llu: %p: %s(%d) : HERE%s\n", DeeSystem_GetWalltime(), DeeThread_Self(), __FILE__, __LINE__,
+	         DeeThread_WasInterrupted(DeeThread_Self()) ? " [INTERRUPT]" : "");
 again:
 	/* Wait for "gc_lock" and "gc_remove_modifying" to settle.
 	 *
 	 * During this part, also try to make it so there's a low
 	 * probability of the GC needing to be reaped once we've
 	 * suspended other threads below! */
 	for (;;) {
-		while (!gc_lock_available() || atomic_read(&gc_remove_modifying))
+		while (!gc_lock_available() || atomic_read(&gc_remove_modifying)) {
+			if (allow_interrupts_and_errors) {
+				if (DeeThread_CheckInterrupt())
+					goto err;
+			} else {
+				DeeThread_CheckInterruptNoInt();
+			}
 			SCHED_YIELD();
+		}
 		if (!atomic_read(&gc_mustreap))
 			break;
-		if (!gc_lock_force_reap(DeeGC_TRACK_F_NOCOLLECT))
-			break;
+		if (!gc_lock_tryacquire())
+			continue;
+		gc_lock_reap_and_release(DeeGC_TRACK_F_NOCOLLECT);
+		break;
 	}
 
 	/* Suspend all other threads (or at least *try* to) */
@@ -861,11 +893,15 @@ gc_collect_acquire(bool allow_interrupts_and_errors) {
 	}
 
 	ASSERT(!DeeThread_IsMultiThreaded);
+	GC_TRACE("%llu: %p: %s(%d) : HERE%s\n", DeeSystem_GetWalltime(), DeeThread_Self(), __FILE__, __LINE__,
+	         DeeThread_WasInterrupted(DeeThread_Self()) ? " [INTERRUPT]" : "");
 	return true;
 resume_threads_and_try_again:
 	DeeThread_ResumeAll();
 	goto again;
 err:
+	GC_TRACE("%llu: %p: %s(%d) : HERE%s\n", DeeSystem_GetWalltime(), DeeThread_Self(), __FILE__, __LINE__,
+	         DeeThread_WasInterrupted(DeeThread_Self()) ? " [INTERRUPT]" : "");
 	return false;
 }
 
@@ -1260,6 +1296,7 @@ continue_with_next_generation:;
 
 	/* Collect memory in GC generations */
 	status = gc_collect_threshold_generations_r(&gc_gen0);
+	GC_TRACE("%llu: %p: %s(%d) : COLLECTED: %u\n", DeeSystem_GetWalltime(), DeeThread_Self(), __FILE__, __LINE__, status);
 	switch (status) {
 	case GC_GENERATION_COLLECT_OR_UNLOCK__NOTHING:
 		/* Nothing could be collected (must objects were moved as they should) */
@@ -1316,10 +1353,11 @@ PRIVATE ATTR_NOINLINE WUNUSED bool DCALL gc_trycollect_gen0(void) {
 			flags |= GC_GENERATION_COLLECT_OR_UNLOCK__F_MOVE_REACHABLE;
 		ASSERT(!DeeThread_IsMultiThreaded);
 		status = gc_generation_collect_or_unlock(iter, &count, flags);
+		GC_TRACE("%llu: %p: %s(%d) : COLLECTED: %u: %lu\n", DeeSystem_GetWalltime(), DeeThread_Self(), __FILE__, __LINE__, status, (unsigned long)count);
 	} else {
-		/* Stop once all overflowing objects have been settled into appropriate generations */
-		status = GC_GENERATION_COLLECT_OR_UNLOCK__SUCCESS;
+		/* All done! */
 		gc_collect_release();
+		return true;
 	}
 	switch (status) {
 	case GC_GENERATION_COLLECT_OR_UNLOCK__NOTHING:

src/deemon/runtime/thread.c

@@ -2266,6 +2266,52 @@ INTERN WUNUSED NONNULL((1)) int
 	return -1;
 }
 
+INTERN NONNULL((1)) void
+(DCALL DeeThread_CheckInterruptNoIntSelf)(DeeThreadObject *__restrict self) {
+#ifdef DeeThread_USE_SINGLE_THREADED
+	(void)self;
+#else /* DeeThread_USE_SINGLE_THREADED */
+	uint32_t state;
+again_read_state:
+	state = atomic_read(&self->t_state);
+
+	/* Check if we're supposed to become suspended. */
+	if ((state & (Dee_THREAD_STATE_SUSPENDING |
+	              Dee_THREAD_STATE_SUSPENDED |
+	              Dee_THREAD_STATE_INTERRUPTED)) ==
+	    (Dee_THREAD_STATE_SUSPENDING | Dee_THREAD_STATE_INTERRUPTED)) {
+		atomic_inc(&self->t_int_vers); /* Indicate that we're checking for interrupts */
+
+		/* Enter the suspend-state */
+		atomic_or(&self->t_state, Dee_THREAD_STATE_SUSPENDED);
+		if (!atomic_cmpxch_weak(&self->t_state, state,
+		                        (state & ~Dee_THREAD_STATE_INTERRUPTED) |
+		                        Dee_THREAD_STATE_SUSPENDED))
+			goto again_read_state;
+		/* Preserve system errors across interrupt checks */
+		DeeSystemError_Push();
+		DeeFutex_WakeAll(&self->t_state);
+
+		/* Wait until the suspend-state ends */
+		for (;;) {
+			state = atomic_read(&self->t_state);
+			if (!(state & Dee_THREAD_STATE_SUSPENDING))
+				break;
+			DeeFutex_Wait32NoInt(&self->t_state, state);
+		}
+
+		/* Leave the suspend state */
+		atomic_and(&self->t_state, ~Dee_THREAD_STATE_SUSPENDED);
+		DeeFutex_WakeAll(&self->t_state);
+
+		/* Restore the system error context */
+		DeeSystemError_Pop();
+		goto again_read_state;
+	}
+#endif /* !DeeThread_USE_SINGLE_THREADED */
+}
+
+
 /* Check for an interrupt exception and throw it if there is one.
  * This function should be called before any blocking system call and is
  * invoked by the interpreter before execution of any JMP-instruction, or
@@ -2275,6 +2321,15 @@ PUBLIC WUNUSED int (DCALL DeeThread_CheckInterrupt)(void) {
 	return DeeThread_CheckInterruptSelf(DeeThread_Self());
 }
 
+/* Same as `DeeThread_CheckInterrupt()', but only handle interrupts that
+ * will not invoke user-code or throw errors. (e.g.: this can be used to
+ * handle thread suspend requests) */
+PUBLIC void (DCALL DeeThread_CheckInterruptNoInt)(void) {
+#ifndef DeeThread_USE_SINGLE_THREADED
+	DeeThread_CheckInterruptNoIntSelf(DeeThread_Self());
+#endif /* !DeeThread_USE_SINGLE_THREADED */
+}
+
 
 #ifndef DeeThread_USE_SINGLE_THREADED
 
@@ -2340,7 +2395,7 @@ forward_appexit_to_main_thread(/*inherit(always)*/ struct Dee_thread_interrupt *
 			atomic_or(&DeeThread_Main.ot_thread.t_state, Dee_THREAD_STATE_WAITING);
 			state |= Dee_THREAD_STATE_WAITING;
 		}
-	
+
 		/* Keep waking the thread in case it just went inside of a blocking system call. */
 		DeeFutex_Wait32NoIntTimed(&DeeThread_Main.ot_thread.t_state, state, THREAD_WAKE_DELAY);
 		if (version != atomic_read(&DeeThread_Main.ot_thread.t_int_vers))
@@ -2552,7 +2607,7 @@ PRIVATE int DeeThread_Entry_func(void *arg)
 			DeeError_Handled(ERROR_HANDLED_INTERRUPT);
 			while (DeeError_Catch(&DeeError_Interrupt))
 				;
-	
+
 			/* If no further exceptions have occurred, set the thread-exit return value. */
 			if (!DeeError_Current()) {
 				_DeeThread_AcquireInterrupt(&self->ot_thread);
@@ -3040,7 +3095,7 @@ DeeThread_Interrupt(/*Thread*/ DeeObject *self,
 				atomic_or(&me->t_state, Dee_THREAD_STATE_WAITING);
 				state |= Dee_THREAD_STATE_WAITING;
 			}
-	
+
 			/* Keep waking the thread in case it just went inside of a blocking system call. */
 			DeeFutex_Wait32NoIntTimed(&me->t_state, state, THREAD_WAKE_DELAY);
 			if (version != atomic_read(&me->t_int_vers))