Re: Changeset 0306c5a64775
Robert Ransom <[email protected]> Sun, 1 Jan 2012 21:41:41 +0000
| Newsgroups | gmane.lisp.scheme.scheme48 |
|---|---|
| Message-ID | <CABqy+srLqXqXgcwBdkzH0PB5OJWL9otYDLsb+_LMf6qwbcvsGg@mail.gmail.com> |
On 2011-12-31, Roderic Morris <[email protected]> wrote: > Right, I noted that and had a similar example when I submitted the patch. > > The external-events package doesn't work as advertised, but I couldn't > figure out how to fix it. It's really the only mechanism that can be > used to do both library calls correctly though. See attached for a patch that fixes one clear bug in the external-events package. That doesn't fix the problem with wait-for-child-process. In order to fix the general problem of incorrect deadlock detection, I think we need: 1. a way to mark thread queues as ‘blocking on an external event’ for deadlock-detection purposes 2. a way to mark condvars and placeholders as ‘blocking on an external event’ (getaddrinfo and wait-for-child-process should be using placeholders; the posix-signals package should be using condvars) 3. support for registering long-term handlers to handle every occurrence of an external event UID 4. an as-general-as-possible ‘external asynchronous result’ system to handle placeholders which should be filled in from C For piece 1 (and 2), we could try iterating through a list of weak references to thread queues to determine whether any threads are blocked on one of the thread queues, but we might need to optimize that data structure if many of those thread queues turn out to not have any threads on them. Robert Ransom
1591-Add-test-case-for-wait-for-child-process.patch
(text/x-diff, 1.3 KB)
# HG changeset patch # User Robert Ransom <[email protected]> # Date 1325425488 28800 # Node ID f43d192876e595bdd98a022de7c4b72f0e098e70 # Parent 080ab15351f6acb9e80efbab54714e7f4505fb3b Add test case for wait-for-child-process diff --git a/scheme/posix/check.scm b/scheme/posix/check.scm --- a/scheme/posix/check.scm +++ b/scheme/posix/check.scm @@ -329,6 +329,29 @@ (list sigusr1)))) (terminate-thread! catch-thread)))) +(define (fork-spawn thunk) + (or (fork) + (begin (thunk) + (exit 0)))) + +(define-syntax fork-and-run + (syntax-rules () + ((fork-and-run body ...) + (fork-spawn (lambda () body ...))))) + +(define-test-case wait-for-child-process posix-core-tests + (let* ((n-waiters 50) + (waiter-results (make-vector n-waiters #f)) + (child-pid (fork-and-run (sleep 5000))) + (waiter-threads + (map (lambda (i) + (spawn (lambda () + (wait-for-child-process child-pid) + (vector-set! waiter-results i #t)))) + (srfi-1:iota n-waiters)))) + (sleep 10000) + (check waiter-results => (make-vector n-waiters #t)))) + ; This should be last, because it removes the directory. (define-test-case rmdir posix-core-tests
1592-Use-an-existing-condvar-if-we-already-have-one-for-a-given-external-event-UID.patch
(text/x-diff, 2.2 KB)
# HG changeset patch # User Robert Ransom <[email protected]> # Date 1325429993 28800 # Node ID dd3edc219b136147f9c5925190cd8384804dd72d # Parent f43d192876e595bdd98a022de7c4b72f0e098e70 Use an existing condvar if we already have one for a given external event UID Previously, wait-for-external-event would add a new condvar to the external-event-condvars list every time it was called, even if we already had a condvar in the list with a given UID. Since the rest of the module would only wake up a single condvar when an external event occurred, this bug could have fouled up the external event system quite severely. diff --git a/scheme/rts/external-event.scm b/scheme/rts/external-event.scm --- a/scheme/rts/external-event.scm +++ b/scheme/rts/external-event.scm @@ -29,13 +29,25 @@ (define (set-external-event-condvars! condvars) (session-data-set! external-events-wait-condvars-slot condvars)) - -; Adding a condvar and uid - the caller has already determined there -; is no existing condvar for this uid. -(define (add-external-event-condvar! uid condvar) - (set-external-event-condvars! (cons (cons uid condvar) - (external-event-condvars)))) +;; Return the condition variable for the specified external event UID, +;; adding a new one to the list if necessary. +;; +;; This function must be called with interrupts disabled. +(define (get-external-event-condvar! uid) + (let loop ((condvars (external-event-condvars))) + (cond + ((null? condvars) + ;; No condvar for this event yet -- create one. + (let ((condvar (make-condvar))) + (set-external-event-condvars! (cons (cons uid condvar) + (external-event-condvars))) + condvar)) + ((= (caar condvars) uid) + ;; Found an existing condvar for this event. + (cdar condvars)) + (else + (loop (cdr condvars)))))) (define (notify-external-event-condvar! condvar) (with-new-proposal (lose) @@ -49,8 +61,7 @@ (define (wait-for-external-event uid) (let ((ints (disable-interrupts!)) - (condvar (make-condvar))) - (add-external-event-condvar! uid condvar) + (condvar (get-external-event-condvar! uid))) (with-new-proposal (lose) (maybe-commit-and-wait-for-condvar condvar)) (set-enabled-interrupts! ints)))