Hi folks,
Maybe someone has a feel for how interrupts work in ABCL?
Doing an interrupt in ABCL from Slime doesn't interrupt until the end of a function or sleep or wait is called. This means whenever I accidentally have a too long or infinite loop by mistake the only thing I can do is quit lisp and start over. This is probably THE most irritating problem I have with ABCL. It doesn't happen often but when it does it completely disrupts work.
Slime handles an interrupt by calling interrupt-thread. Interrupt-thread saves the function and sets a flag. When the interrupted process next checks the flag it will call the interrupt function. At first I thought lisp wasn't checking for an interrupt often enough, but that's not the problem. If I compile (loop for i = 1) I get
public final LispObject execute() { ; do { ; if (!Lisp.interrupted) { ; continue; ; } ; Lisp.handleInterrupt(); ; } while (true); ; }
So why doesn't the interrupt work? Well it seems there are two paths to interrupt. One is via interrupt-thread. Here is the code that sets the flag.
final synchronized void interrupt(LispObject function, LispObject args) { pending = new Cons(args, pending); pending = new Cons(function, pending); threadInterrupted = true; javaThread.interrupt(); }
Note that it is setting thread.threadInterrupted rather than Lisp.interrupted. In fact, the only way to set Lisp.interrupted is via Lisp.setInterrupted() and the only function that calls that is ext:interrupt-lisp. Nothing calls ext:interrupt-lisp. So that nice check within the loop is checking for a signal that currently never arrives.
The javathread.interrupt is only periodically detected by an explicit call or via an InterruptedException that Java will throw when the thread yields or via a check of thread.isInterrupted. The latter is checked in one place, inside a catch, which afaik isn't executed unless there's an error in the thread - it's the "Ignoring uncaught exception" path.
Now, if I call ext:interrupt-lisp from the *inferior-lisp* a break comes up during the infinite loop and I can kill it! This is the desired behavior.
The problem is, I can't figure out where to call (ext:interrupt-lisp) within slime. I tried to put it right after interrupt-thread when that is called in swank::queue-thread-interrupt, which is what is eventually called when you hit C-c in the repl. But that causes an error and kills the slime session. And emacs, for that matter, which needs to be force quit.
I'm thinking that in the case of a control-c we should not use thread-interrupt but rather lisp interrupted. threadInterrupted is thread local and Lisp.interrupted is a global. So technically the control c might not go to the right thread, since all threads are checking the global. But in the situation where it's locked in an infinite loop, there's no other thread doing anything, so the right thing happens. In any case I think that's probably easy to fix by setting another volatile to the thread to be interrupted. In general one would have to worry about a race condition if there were several places that might call interrupt-lisp, but in this case it would only be called with an explicit Control-c and perhaps on a request to kill a thread from the slime threads buffer.
It's an open question as to whether interrupt-thread should take advantage of this as well, but that would require more care to avoid a race condition.
Any help on sorting this would be much appreciated.
Thanks, Alan
To me, this looks overly complicated with all these flags. I never understood why ABCL does all this. Java threads already natively have an interrupt flag. Is there a reason for not using the native Java mechanism? https://docs.oracle.com/javase/tutorial/essential/concurrency/interrupt.html
On Thu, 7 Apr 2022 at 00:13, Alan Ruttenberg alanruttenberg@gmail.com wrote:
Hi folks,
Maybe someone has a feel for how interrupts work in ABCL?
Doing an interrupt in ABCL from Slime doesn't interrupt until the end of a function or sleep or wait is called. This means whenever I accidentally have a too long or infinite loop by mistake the only thing I can do is quit lisp and start over. This is probably THE most irritating problem I have with ABCL. It doesn't happen often but when it does it completely disrupts work.
Slime handles an interrupt by calling interrupt-thread. Interrupt-thread saves the function and sets a flag. When the interrupted process next checks the flag it will call the interrupt function. At first I thought lisp wasn't checking for an interrupt often enough, but that's not the problem. If I compile (loop for i = 1) I get
public final LispObject execute() { ; do { ; if (!Lisp.interrupted) { ; continue; ; } ; Lisp.handleInterrupt(); ; } while (true); ; }
So why doesn't the interrupt work? Well it seems there are two paths to interrupt. One is via interrupt-thread. Here is the code that sets the flag.
final synchronized void interrupt(LispObject function, LispObject args) { pending = new Cons(args, pending); pending = new Cons(function, pending); threadInterrupted = true; javaThread.interrupt(); }Note that it is setting thread.threadInterrupted rather than Lisp.interrupted. In fact, the only way to set Lisp.interrupted is via Lisp.setInterrupted() and the only function that calls that is ext:interrupt-lisp. Nothing calls ext:interrupt-lisp. So that nice check within the loop is checking for a signal that currently never arrives.
The javathread.interrupt is only periodically detected by an explicit call or via an InterruptedException that Java will throw when the thread yields or via a check of thread.isInterrupted. The latter is checked in one place, inside a catch, which afaik isn't executed unless there's an error in the thread - it's the "Ignoring uncaught exception" path.
Now, if I call ext:interrupt-lisp from the *inferior-lisp* a break comes up during the infinite loop and I can kill it! This is the desired behavior.
The problem is, I can't figure out where to call (ext:interrupt-lisp) within slime. I tried to put it right after interrupt-thread when that is called in swank::queue-thread-interrupt, which is what is eventually called when you hit C-c in the repl. But that causes an error and kills the slime session. And emacs, for that matter, which needs to be force quit.
I'm thinking that in the case of a control-c we should not use thread-interrupt but rather lisp interrupted. threadInterrupted is thread local and Lisp.interrupted is a global. So technically the control c might not go to the right thread, since all threads are checking the global. But in the situation where it's locked in an infinite loop, there's no other thread doing anything, so the right thing happens. In any case I think that's probably easy to fix by setting another volatile to the thread to be interrupted. In general one would have to worry about a race condition if there were several places that might call interrupt-lisp, but in this case it would only be called with an explicit Control-c and perhaps on a request to kill a thread from the slime threads buffer.
It's an open question as to whether interrupt-thread should take advantage of this as well, but that would require more care to avoid a race condition.
Any help on sorting this would be much appreciated.
Thanks, Alan
On Apr 7, 2022, at 08:22, Alessio Stalla alessiostalla@gmail.com wrote:
To me, this looks overly complicated with all these flags. I never understood why ABCL does all this. Java threads already natively have an interrupt flag. Is there a reason for not using the native Java mechanism? https://docs.oracle.com/javase/tutorial/essential/concurrency/interrupt.html
I would just wrap the native Java conventions that runs on the current runtime targets (openjdk{8,11,17}), but obviously not use anything that is deprecated.
Our thread model, such as it ever was, is certainly a 1:1 mapping of LispThread to JVM thread concept, that there has never been an idea of N:M native/green thread pooling stuff.
On Apr 7, 2022, at 08:22, Alessio Stalla alessiostalla@gmail.com wrote:
[…] Java threads already natively have an interrupt flag.
Presumably, the [Thread.State enumeration] is what you are referring to. I have checked that it is present in openjdk{8,11,17}, and not marked as deprecated.
[1]: https://docs.oracle.com/javase/8/docs/api/java/lang/Thread.State.html
No, I mean https://docs.oracle.com/javase/8/docs/api/java/lang/Thread.html#interrupted-...
The only deprecated methods are suspend, resume and stop. The interrupt machinery is what developers are supposed to use (at least when not using some higher-level framework). It's a form of cooperative multitasking, since a thread can completely ignore the interrupt. However, given we have control over Lisp threads, we can play nice with that convention, and maybe offer a without-interrupts special form for performance-critical code where the developer assumes the responsibility of checking for interrupts in selected moments.
On Thu, 7 Apr 2022 at 11:42, Mark Evenson evenson@panix.com wrote:
On Apr 7, 2022, at 08:22, Alessio Stalla alessiostalla@gmail.com
wrote:
[…] Java threads already natively have an interrupt flag.
Presumably, the [Thread.State enumeration] is what you are referring to. I have checked that it is present in openjdk{8,11,17}, and not marked as deprecated.
-- "A screaming comes across the sky. It has happened before but there is nothing to compare to it now."
The functions that ABCL uses in Threads.java are thread.interrupt and thread.isInterrupted. It overrides interrupt and isInterrupted to implement interrupt-thread - by saving away the function the target thread is to be interrupted with. Remember, interrupt-thread isn't executed by the thread to be interrupted, it's called from another thread. The receiving thread handles InterruptedException, which is only called on explicit check of interrupt state or from sleep or wait.
Separately, in Lisp.Java there's setInterrupted and handleInterrupt that don't use the java mechanism. Rather setInterrupted sets a static volatile Lisp.interrupted, handleInterrupted clears the variable and calls break. setInterrupted, as I said, is only called from interrupt-lisp. Code generation inserts checks of Lisp.interrupted and calls handleInterrupted if it is set. I have a feeling that this part was done so that frequent interrupt checks could be made without taking the performance hit of a synchronized method call. Remember this check is done inside every loop iteration. The check for Lisp.interrupted is a volatile read, which in most cases is handled in the cache. A write to the volatile invalidates the cache forcing the new value to be read from memory. So much less expensive.
It would be useful to know the history of how it came to be that there are two different interrupt mechanisms.
On Thu, Apr 7, 2022 at 7:21 AM Alessio Stalla alessiostalla@gmail.com wrote:
No, I mean https://docs.oracle.com/javase/8/docs/api/java/lang/Thread.html#interrupted-...
The only deprecated methods are suspend, resume and stop. The interrupt machinery is what developers are supposed to use (at least when not using some higher-level framework). It's a form of cooperative multitasking, since a thread can completely ignore the interrupt. However, given we have control over Lisp threads, we can play nice with that convention, and maybe offer a without-interrupts special form for performance-critical code where the developer assumes the responsibility of checking for interrupts in selected moments.
On Thu, 7 Apr 2022 at 11:42, Mark Evenson evenson@panix.com wrote:
On Apr 7, 2022, at 08:22, Alessio Stalla alessiostalla@gmail.com
wrote:
[…] Java threads already natively have an interrupt flag.
Presumably, the [Thread.State enumeration] is what you are referring to. I have checked that it is present in openjdk{8,11,17}, and not marked as deprecated.
-- "A screaming comes across the sky. It has happened before but there is nothing to compare to it now."
Some more: This is where the code for checking for interrupt is generated (defun maybe-generate-interrupt-check () (unless (> *speed* *safety*) (let ((label1 (gensym)))
* (emit-getstatic +lisp+ "interrupted" :boolean) (emit 'ifeq label1)* (emit-invokestatic +lisp+ "handleInterrupt" nil nil) (label label1))))
I'm thinking, since interrupt-lisp isn't used anyways, modify it to take a thread argument. In interrupt-lisp, call setInterrupted with that thread. In Lisp.java change the type of interrupted and change setInterrupted to take a thread argument, setting Lisp.interrupted to it.
Then, in the maybe-generate-interrupt-check don't Lisp.interrupted check as boolean, check if eq to current thread. The current thread can be made accessible adding (ensure-thread-var-initialized), which puts it into a local at the beginning of the function. Access it with (aload *thread*) which puts it on the stack. I think this means that the lines underlined above get replaced with.
(ensure-thread-var-initialized) (emit-getstatic +lisp+ "interrupted" +lisp-thread+) (aload *thread*) (emit 'if_acmpne label1)
In Lisp.handleInterrupts it's setInterrupted(null) vs setInterrrupted(false);
As stated, this mechanism is strictly to enable control-c.
However, maybe we could have it help thread-interrupt respond more quickly. in Lisp.handleInterrupts. check thread.isInterrupted and call processThreadInterrupts (the thing that is done now) otherwise break. Have the implementation of thread-interrupt do what it does now and also call interrupt-lisp to get it noticed. ---
I haven't tried this. Mostly I'm fumbling around trying to get enough information to do this. I don't know java byte code except what I learned researching this. I don't know if there are pitfalls. I don't know why something like this wouldn't have been done in the first place.
The only thing I can currently think of is that there could be a race condition on setting Lisp.interrupted. The consequence of that is that if there were two quick thread-interrupts, only one of the threads would do the quick response. The other would be processed in the way it currently is.
Anyways, comments solicited. Alan
On Thu, Apr 7, 2022 at 1:21 PM Alan Ruttenberg alanruttenberg@gmail.com wrote:
The functions that ABCL uses in Threads.java are thread.interrupt and thread.isInterrupted. It overrides interrupt and isInterrupted to implement interrupt-thread - by saving away the function the target thread is to be interrupted with. Remember, interrupt-thread isn't executed by the thread to be interrupted, it's called from another thread. The receiving thread handles InterruptedException, which is only called on explicit check of interrupt state or from sleep or wait.
Separately, in Lisp.Java there's setInterrupted and handleInterrupt that don't use the java mechanism. Rather setInterrupted sets a static volatile Lisp.interrupted, handleInterrupted clears the variable and calls break. setInterrupted, as I said, is only called from interrupt-lisp. Code generation inserts checks of Lisp.interrupted and calls handleInterrupted if it is set. I have a feeling that this part was done so that frequent interrupt checks could be made without taking the performance hit of a synchronized method call. Remember this check is done inside every loop iteration. The check for Lisp.interrupted is a volatile read, which in most cases is handled in the cache. A write to the volatile invalidates the cache forcing the new value to be read from memory. So much less expensive.
It would be useful to know the history of how it came to be that there are two different interrupt mechanisms.
On Thu, Apr 7, 2022 at 7:21 AM Alessio Stalla alessiostalla@gmail.com wrote:
No, I mean https://docs.oracle.com/javase/8/docs/api/java/lang/Thread.html#interrupted-...
The only deprecated methods are suspend, resume and stop. The interrupt machinery is what developers are supposed to use (at least when not using some higher-level framework). It's a form of cooperative multitasking, since a thread can completely ignore the interrupt. However, given we have control over Lisp threads, we can play nice with that convention, and maybe offer a without-interrupts special form for performance-critical code where the developer assumes the responsibility of checking for interrupts in selected moments.
On Thu, 7 Apr 2022 at 11:42, Mark Evenson evenson@panix.com wrote:
On Apr 7, 2022, at 08:22, Alessio Stalla alessiostalla@gmail.com
wrote:
[…] Java threads already natively have an interrupt flag.
Presumably, the [Thread.State enumeration] is what you are referring to. I have checked that it is present in openjdk{8,11,17}, and not marked as deprecated.
-- "A screaming comes across the sky. It has happened before but there is nothing to compare to it now."
Ok, I think I cracked it. Check out https://github.com/armedbear/abcl/pull/482 Alan
On Thu, Apr 7, 2022 at 9:26 PM Alan Ruttenberg alanruttenberg@gmail.com wrote:
Some more: This is where the code for checking for interrupt is generated (defun maybe-generate-interrupt-check () (unless (> *speed* *safety*) (let ((label1 (gensym)))
- (emit-getstatic +lisp+ "interrupted" :boolean) (emit 'ifeq label1)* (emit-invokestatic +lisp+ "handleInterrupt" nil nil) (label label1))))
I'm thinking, since interrupt-lisp isn't used anyways, modify it to take a thread argument. In interrupt-lisp, call setInterrupted with that thread. In Lisp.java change the type of interrupted and change setInterrupted to take a thread argument, setting Lisp.interrupted to it.
Then, in the maybe-generate-interrupt-check don't Lisp.interrupted check as boolean, check if eq to current thread. The current thread can be made accessible adding (ensure-thread-var-initialized), which puts it into a local at the beginning of the function. Access it with (aload *thread*) which puts it on the stack. I think this means that the lines underlined above get replaced with.
(ensure-thread-var-initialized) (emit-getstatic +lisp+ "interrupted" +lisp-thread+) (aload *thread*) (emit 'if_acmpne label1)
In Lisp.handleInterrupts it's setInterrupted(null) vs setInterrrupted(false);
As stated, this mechanism is strictly to enable control-c.
However, maybe we could have it help thread-interrupt respond more quickly. in Lisp.handleInterrupts. check thread.isInterrupted and call processThreadInterrupts (the thing that is done now) otherwise break. Have the implementation of thread-interrupt do what it does now and also call interrupt-lisp to get it noticed.
I haven't tried this. Mostly I'm fumbling around trying to get enough information to do this. I don't know java byte code except what I learned researching this. I don't know if there are pitfalls. I don't know why something like this wouldn't have been done in the first place.
The only thing I can currently think of is that there could be a race condition on setting Lisp.interrupted. The consequence of that is that if there were two quick thread-interrupts, only one of the threads would do the quick response. The other would be processed in the way it currently is.
Anyways, comments solicited. Alan
On Thu, Apr 7, 2022 at 1:21 PM Alan Ruttenberg alanruttenberg@gmail.com wrote:
The functions that ABCL uses in Threads.java are thread.interrupt and thread.isInterrupted. It overrides interrupt and isInterrupted to implement interrupt-thread - by saving away the function the target thread is to be interrupted with. Remember, interrupt-thread isn't executed by the thread to be interrupted, it's called from another thread. The receiving thread handles InterruptedException, which is only called on explicit check of interrupt state or from sleep or wait.
Separately, in Lisp.Java there's setInterrupted and handleInterrupt that don't use the java mechanism. Rather setInterrupted sets a static volatile Lisp.interrupted, handleInterrupted clears the variable and calls break. setInterrupted, as I said, is only called from interrupt-lisp. Code generation inserts checks of Lisp.interrupted and calls handleInterrupted if it is set. I have a feeling that this part was done so that frequent interrupt checks could be made without taking the performance hit of a synchronized method call. Remember this check is done inside every loop iteration. The check for Lisp.interrupted is a volatile read, which in most cases is handled in the cache. A write to the volatile invalidates the cache forcing the new value to be read from memory. So much less expensive.
It would be useful to know the history of how it came to be that there are two different interrupt mechanisms.
On Thu, Apr 7, 2022 at 7:21 AM Alessio Stalla alessiostalla@gmail.com wrote:
No, I mean https://docs.oracle.com/javase/8/docs/api/java/lang/Thread.html#interrupted-...
The only deprecated methods are suspend, resume and stop. The interrupt machinery is what developers are supposed to use (at least when not using some higher-level framework). It's a form of cooperative multitasking, since a thread can completely ignore the interrupt. However, given we have control over Lisp threads, we can play nice with that convention, and maybe offer a without-interrupts special form for performance-critical code where the developer assumes the responsibility of checking for interrupts in selected moments.
On Thu, 7 Apr 2022 at 11:42, Mark Evenson evenson@panix.com wrote:
On Apr 7, 2022, at 08:22, Alessio Stalla alessiostalla@gmail.com
wrote:
[…] Java threads already natively have an interrupt flag.
Presumably, the [Thread.State enumeration] is what you are referring to. I have checked that it is present in openjdk{8,11,17}, and not marked as deprecated.
-- "A screaming comes across the sky. It has happened before but there is nothing to compare to it now."
I don't get why ABCL needs a volatile flag where it could just use Thread.interrupted() ( https://docs.oracle.com/javase/8/docs/api/java/lang/Thread.html#interrupted-...). I guess the latter cannot be less efficient than a userland solution like ABCL's.
On Mon, 11 Apr 2022 at 04:32, Alan Ruttenberg alanruttenberg@gmail.com wrote:
Ok, I think I cracked it. Check out https://github.com/armedbear/abcl/pull/482 Alan
On Thu, Apr 7, 2022 at 9:26 PM Alan Ruttenberg alanruttenberg@gmail.com wrote:
Some more: This is where the code for checking for interrupt is generated (defun maybe-generate-interrupt-check () (unless (> *speed* *safety*) (let ((label1 (gensym)))
- (emit-getstatic +lisp+ "interrupted" :boolean) (emit 'ifeq label1)* (emit-invokestatic +lisp+ "handleInterrupt" nil nil) (label label1))))
I'm thinking, since interrupt-lisp isn't used anyways, modify it to take a thread argument. In interrupt-lisp, call setInterrupted with that thread. In Lisp.java change the type of interrupted and change setInterrupted to take a thread argument, setting Lisp.interrupted to it.
Then, in the maybe-generate-interrupt-check don't Lisp.interrupted check as boolean, check if eq to current thread. The current thread can be made accessible adding (ensure-thread-var-initialized), which puts it into a local at the beginning of the function. Access it with (aload *thread*) which puts it on the stack. I think this means that the lines underlined above get replaced with.
(ensure-thread-var-initialized) (emit-getstatic +lisp+ "interrupted" +lisp-thread+) (aload *thread*) (emit 'if_acmpne label1)
In Lisp.handleInterrupts it's setInterrupted(null) vs setInterrrupted(false);
As stated, this mechanism is strictly to enable control-c.
However, maybe we could have it help thread-interrupt respond more quickly. in Lisp.handleInterrupts. check thread.isInterrupted and call processThreadInterrupts (the thing that is done now) otherwise break. Have the implementation of thread-interrupt do what it does now and also call interrupt-lisp to get it noticed.
I haven't tried this. Mostly I'm fumbling around trying to get enough information to do this. I don't know java byte code except what I learned researching this. I don't know if there are pitfalls. I don't know why something like this wouldn't have been done in the first place.
The only thing I can currently think of is that there could be a race condition on setting Lisp.interrupted. The consequence of that is that if there were two quick thread-interrupts, only one of the threads would do the quick response. The other would be processed in the way it currently is.
Anyways, comments solicited. Alan
On Thu, Apr 7, 2022 at 1:21 PM Alan Ruttenberg alanruttenberg@gmail.com wrote:
The functions that ABCL uses in Threads.java are thread.interrupt and thread.isInterrupted. It overrides interrupt and isInterrupted to implement interrupt-thread - by saving away the function the target thread is to be interrupted with. Remember, interrupt-thread isn't executed by the thread to be interrupted, it's called from another thread. The receiving thread handles InterruptedException, which is only called on explicit check of interrupt state or from sleep or wait.
Separately, in Lisp.Java there's setInterrupted and handleInterrupt that don't use the java mechanism. Rather setInterrupted sets a static volatile Lisp.interrupted, handleInterrupted clears the variable and calls break. setInterrupted, as I said, is only called from interrupt-lisp. Code generation inserts checks of Lisp.interrupted and calls handleInterrupted if it is set. I have a feeling that this part was done so that frequent interrupt checks could be made without taking the performance hit of a synchronized method call. Remember this check is done inside every loop iteration. The check for Lisp.interrupted is a volatile read, which in most cases is handled in the cache. A write to the volatile invalidates the cache forcing the new value to be read from memory. So much less expensive.
It would be useful to know the history of how it came to be that there are two different interrupt mechanisms.
On Thu, Apr 7, 2022 at 7:21 AM Alessio Stalla alessiostalla@gmail.com wrote:
No, I mean https://docs.oracle.com/javase/8/docs/api/java/lang/Thread.html#interrupted-...
The only deprecated methods are suspend, resume and stop. The interrupt machinery is what developers are supposed to use (at least when not using some higher-level framework). It's a form of cooperative multitasking, since a thread can completely ignore the interrupt. However, given we have control over Lisp threads, we can play nice with that convention, and maybe offer a without-interrupts special form for performance-critical code where the developer assumes the responsibility of checking for interrupts in selected moments.
On Thu, 7 Apr 2022 at 11:42, Mark Evenson evenson@panix.com wrote:
On Apr 7, 2022, at 08:22, Alessio Stalla alessiostalla@gmail.com
wrote:
[…] Java threads already natively have an interrupt flag.
Presumably, the [Thread.State enumeration] is what you are referring to. I have checked that it is present in openjdk{8,11,17}, and not marked as deprecated.
-- "A screaming comes across the sky. It has happened before but there is nothing to compare to it now."
I'm guessing that Thread.isInterrupted() was avoided because the check is made inside every loop iteration. As a synchronized method I'm guessing it might have been thought to hurt performance. A volatile read is cheap, as I understand it. It's read from cache until a write invalidates a cache line, at which point it becomes a slow read from memory. Even the volatile check is removed when compiling speed high and safety low. I haven't done performance checks since it's the compiler that inserts the checks and I'd rather not mess with the compiler unless I have to.
It's hard to say why the design is the way it is without someone who was around when the compiler was written and understands the rationale chiming in. While I learned enough to connect the two strands of interrupt handling, I don't really understand the design choices. I wouldn't, without further study, understand how to decide where it's appropriate to catch InterruptedException, or where it's safe to handle interrupts. This work is really a patch motivated by me wanting control-c to work more reliably. It's a bonus that it improves interrupt-thread, and I only saw that I might be able to do that towards the end. Similarly I thought I might have to make some slime changes and it's nice that it worked out that I don't have to. It was a relief that I didn't have to mess with the compiler, as that's one more large chunk I don't have a good grasp of.
My main concern at this point is whether the changes introduce any instability. I don't *think* so, but having others review the changes and live with it for a bit would be good.
Alan
I'm talking about Thread.interrupted(), which also resets the flag, not isInterrupted(); anyway, neither of those is a synchronized method. I think this part of ABCL should be made simpler, it would also be easier to reason about it then. Also, Java code invoking Lisp code won't necessarily know about ABCL's thread interrupt protocol, and ABCL's threads may look like they don't play nice with interrupts.
On Mon, 11 Apr 2022 at 07:25, Alan Ruttenberg alanruttenberg@gmail.com wrote:
I'm guessing that Thread.isInterrupted() was avoided because the check is made inside every loop iteration. As a synchronized method I'm guessing it might have been thought to hurt performance. A volatile read is cheap, as I understand it. It's read from cache until a write invalidates a cache line, at which point it becomes a slow read from memory. Even the volatile check is removed when compiling speed high and safety low. I haven't done performance checks since it's the compiler that inserts the checks and I'd rather not mess with the compiler unless I have to.
It's hard to say why the design is the way it is without someone who was around when the compiler was written and understands the rationale chiming in. While I learned enough to connect the two strands of interrupt handling, I don't really understand the design choices. I wouldn't, without further study, understand how to decide where it's appropriate to catch InterruptedException, or where it's safe to handle interrupts. This work is really a patch motivated by me wanting control-c to work more reliably. It's a bonus that it improves interrupt-thread, and I only saw that I might be able to do that towards the end. Similarly I thought I might have to make some slime changes and it's nice that it worked out that I don't have to. It was a relief that I didn't have to mess with the compiler, as that's one more large chunk I don't have a good grasp of.
My main concern at this point is whether the changes introduce any instability. I don't *think* so, but having others review the changes and live with it for a bit would be good.
Alan
Ah, you are right about them not being synchronized. The method on LispThread is, though, which is how I got confused. If you have a proposal for how to make things simpler and are interested in getting it implemented, I'd encourage you to spec it out. I can see about chipping in some effort. Alan
On Mon, Apr 11, 2022 at 1:34 AM Alessio Stalla alessiostalla@gmail.com wrote:
I'm talking about Thread.interrupted(), which also resets the flag, not isInterrupted(); anyway, neither of those is a synchronized method. I think this part of ABCL should be made simpler, it would also be easier to reason about it then. Also, Java code invoking Lisp code won't necessarily know about ABCL's thread interrupt protocol, and ABCL's threads may look like they don't play nice with interrupts.
On Mon, 11 Apr 2022 at 07:25, Alan Ruttenberg alanruttenberg@gmail.com wrote:
I'm guessing that Thread.isInterrupted() was avoided because the check is made inside every loop iteration. As a synchronized method I'm guessing it might have been thought to hurt performance. A volatile read is cheap, as I understand it. It's read from cache until a write invalidates a cache line, at which point it becomes a slow read from memory. Even the volatile check is removed when compiling speed high and safety low. I haven't done performance checks since it's the compiler that inserts the checks and I'd rather not mess with the compiler unless I have to.
It's hard to say why the design is the way it is without someone who was around when the compiler was written and understands the rationale chiming in. While I learned enough to connect the two strands of interrupt handling, I don't really understand the design choices. I wouldn't, without further study, understand how to decide where it's appropriate to catch InterruptedException, or where it's safe to handle interrupts. This work is really a patch motivated by me wanting control-c to work more reliably. It's a bonus that it improves interrupt-thread, and I only saw that I might be able to do that towards the end. Similarly I thought I might have to make some slime changes and it's nice that it worked out that I don't have to. It was a relief that I didn't have to mess with the compiler, as that's one more large chunk I don't have a good grasp of.
My main concern at this point is whether the changes introduce any instability. I don't *think* so, but having others review the changes and live with it for a bit would be good.
Alan
OK, I'll give it a shot as soon as I can.
On Mon, 11 Apr 2022 at 18:58, Alan Ruttenberg alanruttenberg@gmail.com wrote:
Ah, you are right about them not being synchronized. The method on LispThread is, though, which is how I got confused. If you have a proposal for how to make things simpler and are interested in getting it implemented, I'd encourage you to spec it out. I can see about chipping in some effort. Alan
On Mon, Apr 11, 2022 at 1:34 AM Alessio Stalla alessiostalla@gmail.com wrote:
I'm talking about Thread.interrupted(), which also resets the flag, not isInterrupted(); anyway, neither of those is a synchronized method. I think this part of ABCL should be made simpler, it would also be easier to reason about it then. Also, Java code invoking Lisp code won't necessarily know about ABCL's thread interrupt protocol, and ABCL's threads may look like they don't play nice with interrupts.
On Mon, 11 Apr 2022 at 07:25, Alan Ruttenberg alanruttenberg@gmail.com wrote:
I'm guessing that Thread.isInterrupted() was avoided because the check is made inside every loop iteration. As a synchronized method I'm guessing it might have been thought to hurt performance. A volatile read is cheap, as I understand it. It's read from cache until a write invalidates a cache line, at which point it becomes a slow read from memory. Even the volatile check is removed when compiling speed high and safety low. I haven't done performance checks since it's the compiler that inserts the checks and I'd rather not mess with the compiler unless I have to.
It's hard to say why the design is the way it is without someone who was around when the compiler was written and understands the rationale chiming in. While I learned enough to connect the two strands of interrupt handling, I don't really understand the design choices. I wouldn't, without further study, understand how to decide where it's appropriate to catch InterruptedException, or where it's safe to handle interrupts. This work is really a patch motivated by me wanting control-c to work more reliably. It's a bonus that it improves interrupt-thread, and I only saw that I might be able to do that towards the end. Similarly I thought I might have to make some slime changes and it's nice that it worked out that I don't have to. It was a relief that I didn't have to mess with the compiler, as that's one more large chunk I don't have a good grasp of.
My main concern at this point is whether the changes introduce any instability. I don't *think* so, but having others review the changes and live with it for a bit would be good.
Alan
ABCL does use the native mechanism to implement interrupt-thread. The control thread receives save the interrupt-thread function, sets a flag, and then uses java's thread.interrupt on the targeted thread. The problem is that the handling of the interrupt in the targeted thread needs to be done by that thread and when the thread is busy it won't handle the interrupt. I think that's the intent including the checks for interrupt in generated code - so that it might be handled sooner. The problem is that check isn't checking the right variable.
On Thu, Apr 7, 2022 at 3:23 AM Alessio Stalla alessiostalla@gmail.com wrote:
To me, this looks overly complicated with all these flags. I never understood why ABCL does all this. Java threads already natively have an interrupt flag. Is there a reason for not using the native Java mechanism? https://docs.oracle.com/javase/tutorial/essential/concurrency/interrupt.html
On Thu, 7 Apr 2022 at 00:13, Alan Ruttenberg alanruttenberg@gmail.com wrote:
Hi folks,
Maybe someone has a feel for how interrupts work in ABCL?
Doing an interrupt in ABCL from Slime doesn't interrupt until the end of a function or sleep or wait is called. This means whenever I accidentally have a too long or infinite loop by mistake the only thing I can do is quit lisp and start over. This is probably THE most irritating problem I have with ABCL. It doesn't happen often but when it does it completely disrupts work.
Slime handles an interrupt by calling interrupt-thread. Interrupt-thread saves the function and sets a flag. When the interrupted process next checks the flag it will call the interrupt function. At first I thought lisp wasn't checking for an interrupt often enough, but that's not the problem. If I compile (loop for i = 1) I get
public final LispObject execute() { ; do { ; if (!Lisp.interrupted) { ; continue; ; } ; Lisp.handleInterrupt(); ; } while (true); ; }
So why doesn't the interrupt work? Well it seems there are two paths to interrupt. One is via interrupt-thread. Here is the code that sets the flag.
final synchronized void interrupt(LispObject function, LispObjectargs) { pending = new Cons(args, pending); pending = new Cons(function, pending); threadInterrupted = true; javaThread.interrupt(); }
Note that it is setting thread.threadInterrupted rather than Lisp.interrupted. In fact, the only way to set Lisp.interrupted is via Lisp.setInterrupted() and the only function that calls that is ext:interrupt-lisp. Nothing calls ext:interrupt-lisp. So that nice check within the loop is checking for a signal that currently never arrives.
The javathread.interrupt is only periodically detected by an explicit call or via an InterruptedException that Java will throw when the thread yields or via a check of thread.isInterrupted. The latter is checked in one place, inside a catch, which afaik isn't executed unless there's an error in the thread - it's the "Ignoring uncaught exception" path.
Now, if I call ext:interrupt-lisp from the *inferior-lisp* a break comes up during the infinite loop and I can kill it! This is the desired behavior.
The problem is, I can't figure out where to call (ext:interrupt-lisp) within slime. I tried to put it right after interrupt-thread when that is called in swank::queue-thread-interrupt, which is what is eventually called when you hit C-c in the repl. But that causes an error and kills the slime session. And emacs, for that matter, which needs to be force quit.
I'm thinking that in the case of a control-c we should not use thread-interrupt but rather lisp interrupted. threadInterrupted is thread local and Lisp.interrupted is a global. So technically the control c might not go to the right thread, since all threads are checking the global. But in the situation where it's locked in an infinite loop, there's no other thread doing anything, so the right thing happens. In any case I think that's probably easy to fix by setting another volatile to the thread to be interrupted. In general one would have to worry about a race condition if there were several places that might call interrupt-lisp, but in this case it would only be called with an explicit Control-c and perhaps on a request to kill a thread from the slime threads buffer.
It's an open question as to whether interrupt-thread should take advantage of this as well, but that would require more care to avoid a race condition.
Any help on sorting this would be much appreciated.
Thanks, Alan
armedbear-devel@common-lisp.net
-
Alan Ruttenberg -
Alessio Stalla -
Mark Evenson