From 4ea09bf731b8a3c5849471d1b2e529b64d172710 Mon Sep 17 00:00:00 2001
From: Matthew Flatt <mflatt@racket-lang.org>
Date: Mon, 7 Oct 2019 19:05:45 -0600
Subject: [PATCH] cs & thread: fix shared hash table

An `eq?`-based hash table in the implementation of custodians was
still shared across threads.

Also, taking the global lock at the Rumble level did not disable
interrupts. Since sometimes the lock is taken with interrupts
disabled, threads could potentially deadlock by not having an order.
Fix the problem by disabling interrupts before taking the lock.
---
 racket/src/cs/rumble/lock.ss    | 22 +++++++++++++++++-----
 racket/src/thread/custodian.rkt |  5 +++--
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/racket/src/cs/rumble/lock.ss b/racket/src/cs/rumble/lock.ss
index 99ddbdeea6..8edbdb0dea 100644
--- a/racket/src/cs/rumble/lock.ss
+++ b/racket/src/cs/rumble/lock.ss
@@ -9,16 +9,23 @@
   (set! scheduler-lock-acquire acquire)
   (set! scheduler-lock-release release))
 
+;; Use `with-global-lock` to guard against both engine-based
+;; concurrency and Scheme thread concurrency.
+
+;; Use `with-global-lock*` when no guard against engine-based
+;; concurrency is needed (because some operation is already atomic at
+;; that level).
+
+;; Taking the global lock disables GC, as does even waiting on the
+;; lock. So, of course, make sure all globally-locked actions are
+;; short.
+
 (meta-cond
  [(not (threaded?))
   ;; Using a Chez Scheme build without thread support,
   ;; but we need to cooperate with engine-based threads.
-  ;; Use `with-global-lock*` when no lock is needed absent threads
   (define-syntax-rule (with-global-lock* e ...)
     (begin e ...))
-
-  ;; Use `with-global-lock` when a lock is needed to prevent
-  ;; engine-based concurrency
   (define-syntax-rule (with-global-lock e ...)
     (with-interrupts-disabled
      e ...))]
@@ -30,11 +37,16 @@
   (define-syntax-rule (with-global-lock* e ...)
     (with-global-lock e ...))
   (define-syntax-rule (with-global-lock e ...)
+    ;; Disable interrupts before taking the lock, because some threads
+    ;; may have interrupts disabled when they take the lock (which
+    ;; blocks GCs), so always disabling interrupts imposes an order.
     (begin
+      (disable-interrupts)
       (mutex-acquire global-lock)
       (begin0
        (begin e ...)
-       (mutex-release global-lock))))])
+       (mutex-release global-lock)
+       (enable-interrupts))))])
 
 ;; ------------------------------------------------------------
 ;; Locks used for hash tables
diff --git a/racket/src/thread/custodian.rkt b/racket/src/thread/custodian.rkt
index bf116cdde2..ed31f76af5 100644
--- a/racket/src/thread/custodian.rkt
+++ b/racket/src/thread/custodian.rkt
@@ -82,7 +82,8 @@
 (define (set-root-custodian! c)
   (set! root-custodian c)
   (current-custodian c)
-  (set! custodian-will-executor (host:make-late-will-executor void #f)))
+  (set! custodian-will-executor (host:make-late-will-executor void #f))
+  (set! custodians-with-limits (make-hasheq)))
 
 (define/who (make-custodian [parent (current-custodian)])
   (check who custodian? parent)
@@ -374,7 +375,7 @@
 
 ;; Ensures that custodians with memory limits are not treated as
 ;; inaccessible and merged:
-(define custodians-with-limits (make-hasheq))
+(define-place-local custodians-with-limits (make-hasheq))
 
 ;; ----------------------------------------