I see some new changes in swank-lispworks.lisp where mp:with-lock is used inside mp:without-interrupts. What is the purpose of that?
It is wrong because it will lead to deadlock if the lock is held by another thread (you can't wait if you are inside without-interrupts). I suggest removing the without-interrupts because the lock does is better anyway.
* Martin Simmons [2008-08-05 21:26+0200] writes:
I see some new changes in swank-lispworks.lisp where mp:with-lock is used inside mp:without-interrupts. What is the purpose of that?
My reasoning with this code was:
(loop (mp:process-wait "receive-if" (lambda () (some test (mailbox.queue mbox)))) (mp:without-interrupts (mp:with-lock (lock "receive-if/try" 0.1) (let* ((q (mailbox.queue mbox)) (tail (member-if test q))) (when tail (setf (mailbox.queue mbox) (nconc (ldiff q tail) (cdr tail))) (return (car tail)))))))
there would be no guarantee that the part inside WITH-LOCK isn't interrupted, e.g. by PROCESS-INTERRUPT to run the SLIME debugger. But the debugger, sooner or later, calls RECEIVE-IF in the same thread; recursively so to say. It could happen that the debugger removes the the element that the suspended call was about to remove, i.e. both calls would, after the debugger resumes, return the same element.
Disabling interrupts inside the WITH-LOCK didn't seem right either, because then the interrupt handler would be delayed, but still be run in the dynamic extend of the lock and so blocking any sender.
That's pretty unfortunate mix of threading and interrupts. The current variant with the timeout appeared to me as the least broken. But I would love to learn how to do this properly.
Helmut.
On Tue, 05 Aug 2008 22:51:05 +0200, Helmut Eller said:
- Martin Simmons [2008-08-05 21:26+0200] writes:
I see some new changes in swank-lispworks.lisp where mp:with-lock is used inside mp:without-interrupts. What is the purpose of that?
My reasoning with this code was:
(loop (mp:process-wait "receive-if" (lambda () (some test (mailbox.queue mbox)))) (mp:without-interrupts (mp:with-lock (lock "receive-if/try" 0.1) (let* ((q (mailbox.queue mbox)) (tail (member-if test q))) (when tail (setf (mailbox.queue mbox) (nconc (ldiff q tail) (cdr tail))) (return (car tail)))))))
there would be no guarantee that the part inside WITH-LOCK isn't interrupted, e.g. by PROCESS-INTERRUPT to run the SLIME debugger. But the debugger, sooner or later, calls RECEIVE-IF in the same thread; recursively so to say. It could happen that the debugger removes the the element that the suspended call was about to remove, i.e. both calls would, after the debugger resumes, return the same element.
Disabling interrupts inside the WITH-LOCK didn't seem right either, because then the interrupt handler would be delayed, but still be run in the dynamic extend of the lock and so blocking any sender.
That's pretty unfortunate mix of threading and interrupts. The current variant with the timeout appeared to me as the least broken. But I would love to learn how to do this properly.
Hmmm, I see the problem now.
I think you could implement it without blocking all interrupts by maintaining your own interrupt queue, something like this:
(defvar *pending-slime-interrupts* nil) (defvar *in-without-slime-interrupts* nil)
(defmacro without-slime-interrupts (&body body) `(let ((*pending-slime-interrupts* nil) (*in-without-slime-interrupts* t)) (unwind-protect (progn ,@body) (when *pending-slime-interrupts* (let ((*in-without-slime-interrupts* nil)) (mapc 'funcall *pending-slime-interrupts*))))))
(defun invoke-or-queue-interrupt (function) (if *in-without-slime-interrupts* (push function *pending-slime-interrupts*) (funcall function)))
Then the SLIME interrupt function should call INVOKE-OR-QUEUE-INTERRUPT, which will delay the interrupt if inside the WITHOUT-SLIME-INTERRUPTS form.
* Martin Simmons [2008-08-06 13:33+0200] writes:
Then the SLIME interrupt function should call INVOKE-OR-QUEUE-INTERRUPT, which will delay the interrupt if inside the WITHOUT-SLIME-INTERRUPTS form.
Yes, this sounds much more manageable. Thank you very much for the explanation!
Helmut.
On Tue, Aug 5, 2008 at 11:51 PM, Helmut Eller heller@common-lisp.net wrote:
- Martin Simmons [2008-08-05 21:26+0200] writes:
I see some new changes in swank-lispworks.lisp where mp:with-lock is used inside mp:without-interrupts. What is the purpose of that?
We talked briefly about a related issue in ECLM this year. :)
That's pretty unfortunate mix of threading and interrupts. The current variant with the timeout appeared to me as the least broken. But I would love to learn how to do this properly.
In SBCL The Right Thing would be:
(let (got-it) (without-interrupts (unwind-protect (when (setf got-it (allow-with-interrupts (get-mutex lock))) ...frob-queue...) (when got-it (release-lock lock)))))
and GET-LOCK has the wait wrapped in WITH-INTERRUPTS.
WITH-INTERRUPTS enabled interrupts only if there is an active ALLOW-WITH-INTERRUPTS lexically nested inside each WITHOUT-INTERRUPTS currently on stack. (This is what I was trying to explain while slightly too drunk to make a great deal of sense.)
So: frobbing the queueu is protected from interrupts, and unwinds are protected as well, as is the actual action of grabbing the lock -- but waits can be interrupt.
There is are a couple of internal variants as well, which don't enable interrupts at all, etc -- which are used only when the wait is (we hope!) known to be finite, and the wrapped code is also known to finish very shortly. (Or when interrupts are a really bad idea anyways.)
Cheers,
-- Nikodemus
* Nikodemus Siivola [2008-08-06 13:35+0200] writes:
In SBCL The Right Thing would be:
(let (got-it) (without-interrupts (unwind-protect (when (setf got-it (allow-with-interrupts (get-mutex lock))) ...frob-queue...) (when got-it (release-lock lock)))))
and GET-LOCK has the wait wrapped in WITH-INTERRUPTS.
You meant GET-MUTEX, right?
WITH-INTERRUPTS enabled interrupts only if there is an active ALLOW-WITH-INTERRUPTS lexically nested inside each WITHOUT-INTERRUPTS currently on stack. (This is what I was trying to explain while slightly too drunk to make a great deal of sense.)
Would the following also work?
(without-interrupts (allow-with-interrupts (with-mutex (lock) ...)
Assuming that WITH-MUTEX expands to the ordinary GET-MUTEX/RELEASE-MUTEX pair.
Helmut.
On Wed, Aug 6, 2008 at 5:30 PM, Helmut Eller heller@common-lisp.net wrote:
- Nikodemus Siivola [2008-08-06 13:35+0200] writes:
and GET-LOCK has the wait wrapped in WITH-INTERRUPTS.
You meant GET-MUTEX, right?
Yes, (and it should be RELEASE-MUTEX in the code).
WITH-INTERRUPTS enabled interrupts only if there is an active ALLOW-WITH-INTERRUPTS lexically nested inside each WITHOUT-INTERRUPTS currently on stack. (This is what I was trying to explain while slightly too drunk to make a great deal of sense.)
Would the following also work?
(without-interrupts (allow-with-interrupts (with-mutex (lock) ...)
Assuming that WITH-MUTEX expands to the ordinary GET-MUTEX/RELEASE-MUTEX pair.
Yes.
WITH-MUTEX is essentially the code below, except that FROB-QUEUE bit is wrapped in WITH-LOCAL-INTERRUPTS (which is essentially ALLOW-WITH-INTERRUPTS+WITH-INTERRUPTS).
(let (got-it) (without-interrupts (unwind-protect (when (setf got-it (allow-with-interrupts (get-mutex lock))) ...frob-queue...) (when got-it (release-lock lock)))))
If this was performance sensitive code you'd want to write avoid the extra WITHOUT-INTERRUPTS, though.
Cheers,
-- Nikodemus