suzanne.soy/racket
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

expander: different repair for race

Browse Source 
The repair in 49a90ba75e reorders two lines in a way that, in
retrospect, seems worrying. I can't construct an example that goes
wrong, so maybe it's fine, but it seems possible (now or with some
future change) that attempting to visit available modules could lead
to the same attempt in the same thread and therefore a loop.

This commit changes the repair to just always take the lock instead of
fixing up the attempted shortcut. There's a tiny extra cost to always
taking the lock, but that extra cost seems like a better choice
overall.
This commit is contained in:
Matthew Flatt 2018-07-18 13:31:43 -06:00
parent 49a90ba75e
commit e2435d7187
2 changed files with 16 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
racket/src/expander/namespace/module.rkt
View File

@ -494,19 +494,17 @@
(namespace-run-available-modules! ns (add1 run-phase))) (namespace-run-available-modules! ns (add1 run-phase)))
(define (namespace-run-available-modules! ns [run-phase (namespace-phase ns)]) (define (namespace-run-available-modules! ns [run-phase (namespace-phase ns)])
;; For a quick check, we can rely on the atomicity of `eqv?`-based hash tables
(unless (null? (hash-ref (namespace-available-module-instances ns) run-phase null))
(registry-call-with-lock (registry-call-with-lock
(namespace-module-registry ns) (namespace-module-registry ns)
(lambda () (lambda ()
(let loop () (let loop ()
(define mis (hash-ref (namespace-available-module-instances ns) run-phase null)) (define mis (hash-ref (namespace-available-module-instances ns) run-phase null))
(unless (null? mis) (unless (null? mis)
(hash-set! (namespace-available-module-instances ns) run-phase null)
(for ([mi (in-list (reverse mis))]) (for ([mi (in-list (reverse mis))])
(run-module-instance! mi ns #:run-phase run-phase #:skip-run? #f #:otherwise-available? #f)) (run-module-instance! mi ns #:run-phase run-phase #:skip-run? #f #:otherwise-available? #f))
(hash-set! (namespace-available-module-instances ns) run-phase null)
;; In case instantiation added more reflectively: ;; In case instantiation added more reflectively:
(loop))))))) (loop))))))
(define (namespace-primitive-module-visit! ns name) (define (namespace-primitive-module-visit! ns name)
(define mi (hash-ref (namespace-module-instances ns) (make-resolved-module-path name))) (define mi (hash-ref (namespace-module-instances ns) (make-resolved-module-path name)))

13
racket/src/racket/src/startup.inc
View File

@ -14000,9 +14000,6 @@ static const char *startup_source =
"(let-values(((run-phase_4)" "(let-values(((run-phase_4)"
"(if(eq? run-phase132_0 unsafe-undefined)(namespace-phase ns_37) run-phase132_0)))" "(if(eq? run-phase132_0 unsafe-undefined)(namespace-phase ns_37) run-phase132_0)))"
"(let-values()" "(let-values()"
"(if(null?(hash-ref(namespace-available-module-instances ns_37) run-phase_4 null))"
"(void)"
"(let-values()"
"(registry-call-with-lock" "(registry-call-with-lock"
"(namespace-module-registry$1 ns_37)" "(namespace-module-registry$1 ns_37)"
"(lambda()" "(lambda()"
@ -14019,6 +14016,10 @@ static const char *startup_source =
"(void)" "(void)"
"(let-values()" "(let-values()"
"(begin" "(begin"
"(hash-set!"
"(namespace-available-module-instances ns_37)"
" run-phase_4"
" null)"
"(let-values(((lst_67)(reverse$1 mis_0)))" "(let-values(((lst_67)(reverse$1 mis_0)))"
"(begin" "(begin"
"(if(variable-reference-from-unsafe?" "(if(variable-reference-from-unsafe?"
@ -14068,12 +14069,8 @@ static const char *startup_source =
" for-loop_91)" " for-loop_91)"
" lst_67)))" " lst_67)))"
"(void)" "(void)"
"(hash-set!"
"(namespace-available-module-instances ns_37)"
" run-phase_4"
" null)"
"(loop_77)))))))))" "(loop_77)))))))))"
" loop_77)))))))))))))" " loop_77)))))))))))"
"(case-lambda" "(case-lambda"
"((ns_38)(begin(namespace-run-available-modules!134_0 ns_38 unsafe-undefined)))" "((ns_38)(begin(namespace-run-available-modules!134_0 ns_38 unsafe-undefined)))"
"((ns_39 run-phase132_1)(namespace-run-available-modules!134_0 ns_39 run-phase132_1)))))" "((ns_39 run-phase132_1)(namespace-run-available-modules!134_0 ns_39 run-phase132_1)))))"