RFC: Consolidate stack-unwinding code
Bob Rogers
code into Continuation:invoke; previously, RetContinuation:invoke and
find_exception_handler also did stack-unwinding, and none of the three
did it quite the same way.
Here are the effects:
1. Improved code sharing, a prerequisite for improving semantics.
2. Actions are now triggered when a continuation is called, fixing
two TODO cases (and also two others which were looking for the buggy
behavior). Previously, this only worked for exception handling and
RetContinuation.
3. Actions are no longer triggered as a side-effect of discovering
that there are no active exception handlers, which is more in the spirit
of PDD23.
4. Variable names are now more consistent between
Continuation:invoke and RetContinuation:invoke (or what's left of it).
Questions:
1. I notice that parameter passing is handled by Continuation:invoke
but not by RetContinuation:invoke. Currently I have to use some
cumbersome "Am I a RetContinuation?" logic in what is supposed to be the
generic method in order to deal with this. It would be nicer if they
both passed parameters in the same way. Is there some reason this can't
be done?
2. The exception handling case is not as efficient as it could be
(quadratic handler search followed by re-searching for the handler's
stack entry). This would be helped by splitting the unwinding code into
a separate routine, and calling it from each of the invoke methods,
rather than trying to use inheritance. But I assume this will need to
be rewritten anyway when PDD23 is fully implemented. With that in mind,
is this OK for now?
3. Around src/exceptions.c:268 (unpatched) there is an assignment of
e->entry_type to NO_STACK_ENTRY_TYPE, after a comment explaining why.
The patch disables this assignment, with no ill effect. If the code is
still needed for some reason, it will have to be moved to
RetContinuation:invoke, but I don't have a test case to ensure that I've
done this right. Should I just chuck it?
Suggestions are cordially invited. If I hear no complaints by
tomorrow night, I will assume all answers are "yes," and commit this (or
something close to it).
-- Bob Rogers
http://rgrjr.dyndns.org/
Leopold Toetsch
> The attached patch consolidates most of the existing stack-unwinding
> code into Continuation:invoke; previously, RetContinuation:invoke and
> find_exception_handler also did stack-unwinding, and none of the three
> did it quite the same way.
Very good.
> Here are the effects:
>
> 1. Improved code sharing, a prerequisite for improving semantics.
>
> 2. Actions are now triggered when a continuation is called, fixing
> two TODO cases (and also two others which were looking for the buggy
> behavior). Previously, this only worked for exception handling and
> RetContinuation.
There are (ought to be) presumably 2 different kinds of continuations:
1) I leave this call frame for now, but I'll be back
2) I'm done with this call frame forever
An exit_handler action would be invoked only for the 2nd case.
And while at it, we probably want some more specialized (limited)
continuations with e.g. a given range of call frames, which the continuation
will not leave.
> 3. Actions are no longer triggered as a side-effect of discovering
> that there are no active exception handlers, which is more in the spirit
> of PDD23.
IMHO there is always an exception handler, assuming a toplevel default
handler, which is implicitely handling the "No exception handler found" case.
Avoiding this side-effect seems to be the reason for the very complex and
costly search for active handlers, which I really don't like.
Re PDD23 see below.
> 4. Variable names are now more consistent between
> Continuation:invoke and RetContinuation:invoke (or what's left of it).
Great.
> Questions:
>
> 1. I notice that parameter passing is handled by Continuation:invoke
> but not by RetContinuation:invoke. Currently I have to use some
> cumbersome "Am I a RetContinuation?" logic in what is supposed to be the
> generic method in order to deal with this. It would be nicer if they
> both passed parameters in the same way. Is there some reason this can't
> be done?
Currently set_returns and set_args are 2 different opcodes:
.return (foo, bar) # RetContinuation.invoke - set_returns
cc(foo, bar) # Continuation.invoke - set_args
But with a few changes, we could unify "results" value handling:
- the current_results pointer *should* be part of the continuation structure
and not of the context like now (it is already in the Continuation)
- then unifying set_returns and set_args would give us common
{Ret,}Continuation argument->results passing.
> 2. The exception handling case is not as efficient as it could be
> (quadratic handler search followed by re-searching for the handler's
> stack entry). This would be helped by splitting the unwinding code into
> a separate routine, and calling it from each of the invoke methods,
> rather than trying to use inheritance. But I assume this will need to
> be rewritten anyway when PDD23 is fully implemented. With that in mind,
> is this OK for now?
Given that it'll be rewritten yes ;-)
> 3. Around src/exceptions.c:268 (unpatched) there is an assignment of
> e->entry_type to NO_STACK_ENTRY_TYPE, after a comment explaining why.
> The patch disables this assignment, with no ill effect. If the code is
> still needed for some reason, it will have to be moved to
> RetContinuation:invoke, but I don't have a test case to ensure that I've
> done this right. Should I just chuck it?
It was IIRC needed for DOD. I.e. at some time there was a stale stack entry,
which caused troubles during stack marking. This could have been due to an
error elsewhere of course.
> Suggestions are cordially invited. If I hear no complaints by
> tomorrow night, I will assume all answers are "yes," and commit this (or
> something close to it).
Ok. BTW:
+ && ! ret_continuation_p) {
fprintf(stderr, "[oops; continuation %p of type %d "
"is trying to jump from runloop %d to runloop %d]\n",
This meessage is triggered for exceptions too, which is wrong. Only
continuations that *would* possibly resume this runloop are a problem (i.e.
the 1) case above).
Re PDD23: It's not implementable the way it is. The main problem is of course
the already discussed continuation border of C code. E.g.
<cite> The embedding program
may choose to handle the exception and continue execution by invoking
the exception's continuation.
</cite>
But there's no way currently to safely invoke the continuation from C code and
continue at some arbitrary runloop nesting.
I also dislike the stated mandantory closure-ish behavior of handlers. The
general case seems to be that the handler is a label in the context of the
handling call frame. If HLL semantics are different, the handler code can
still create and call a closure for handling the exception.
And I don't get the rational of:
<cite>
3 Invoke the handler (note: this is still in the thrower's dynamic con-
text).
</cite>
To me it seems that this should always be the exception handler's context.
> -- Bob Rogers
> http://rgrjr.dyndns.org/
leo
Bob Rogers
Date: Mon, 18 Sep 2006 11:53:36 +0200
Am Montag, 18. September 2006 03:56 schrieb Bob Rogers:
> The attached patch consolidates most of the existing stack-unwinding
> code into Continuation:invoke; previously, RetContinuation:invoke and
> find_exception_handler also did stack-unwinding, and none of the three
> did it quite the same way.
Very good.
Thanks.
> Here are the effects:
>
> 1. Improved code sharing, a prerequisite for improving semantics.
>
> 2. Actions are now triggered when a continuation is called, fixing
> two TODO cases (and also two others which were looking for the buggy
> behavior). Previously, this only worked for exception handling and
> RetContinuation.
There are (ought to be) presumably 2 different kinds of continuations:
1) I leave this call frame for now, but I'll be back
2) I'm done with this call frame forever
An exit_handler action would be invoked only for the 2nd case.
Currently, a boolean is passed that is true for an Error_Handler exit
and false otherwise. I think it would be more elegant if the
contination object itself was passed, and then implementers could do
'isa' testing to decide for themselves how to handle each case.
Beyond that, the fact that a given continuation is done with a call
frame forever doesn't mean that there isn't somebody else who might be
able to return to it. In such a case, Parrot can't know which is
supposed to be the last such exit. So it seems more versatile to always
invoke it and let it decide for itself.
And while at it, we probably want some more specialized (limited)
continuations with e.g. a given range of call frames, which the continuation
will not leave.
I'm not sure I see the advantage -- but that may depend on the
definition of "a given range".
> 3. Actions are no longer triggered as a side-effect of discovering
> that there are no active exception handlers, which is more in the spirit
> of PDD23.
IMHO there is always an exception handler, assuming a toplevel default
handler, which is implicitely handling the "No exception handler found" case.
Avoiding this side-effect seems to be the reason for the very complex and
costly search for active handlers, which I really don't like.
It need not be complex or costly; see below.
And this is not the only side effect it avoids. Currently, the only
way a handler can decline to handle a given error is to rethrow it,
destroying the more interesting parts of the stack. When the last
handler rethrows, the default handler can only show the backtrace from
there, which is not likely to be very helpful. If the default handler
were implemented explicitly, as if there were a C<push_eh> before
calling the main sub, there wouldn't be *anything* to backtrace.
IMHO, the "default handler" for an interactive session should be the
debugger; for maximum utility, the debugger should have access to the
pristine state at the point of the error. This is easy as long as
handlers avoid side-effects for errors they do not plan to handle.
> Questions:
>
> 1. I notice that parameter passing is handled by Continuation:invoke
> but not by RetContinuation:invoke. Currently I have to use some
> cumbersome "Am I a RetContinuation?" logic in what is supposed to be the
> generic method in order to deal with this. It would be nicer if they
> both passed parameters in the same way. Is there some reason this can't
> be done?
Currently set_returns and set_args are 2 different opcodes:
.return (foo, bar) # RetContinuation.invoke - set_returns
cc(foo, bar) # Continuation.invoke - set_args
But with a few changes, we could unify "results" value handling:
- the current_results pointer *should* be part of the continuation structure
and not of the context like now (it is already in the Continuation)
- then unifying set_returns and set_args would give us common
{Ret,}Continuation argument->results passing.
That sounds good. It would be even better if this made it easy for C
code to intercept returns via a continuation. But perhaps that is
covered by your Cfunc.pod proposal?
> 2. The exception handling case is not as efficient as it could be
> (quadratic handler search followed by re-searching for the handler's
> stack entry). This would be helped by splitting the unwinding code into
> a separate routine, and calling it from each of the invoke methods,
> rather than trying to use inheritance. But I assume this will need to
> be rewritten anyway when PDD23 is fully implemented. With that in mind,
> is this OK for now?
Given that it'll be rewritten yes ;-)
Good; thanks.
> 3. Around src/exceptions.c:268 (unpatched) there is an assignment of
> e->entry_type to NO_STACK_ENTRY_TYPE, after a comment explaining why.
> The patch disables this assignment, with no ill effect. If the code is
> still needed for some reason, it will have to be moved to
> RetContinuation:invoke, but I don't have a test case to ensure that I've
> done this right. Should I just chuck it?
It was IIRC needed for DOD. I.e. at some time there was a stale stack entry,
which caused troubles during stack marking. This could have been due to an
error elsewhere of course.
Hmm. Then maybe I should do more testing. I'll try throwing random
"sweep 1" calls in test cases that do C<pushaction>.
> Suggestions are cordially invited. If I hear no complaints by
> tomorrow night, I will assume all answers are "yes," and commit this (or
> something close to it).
Ok. BTW:
+ && ! ret_continuation_p) {
fprintf(stderr, "[oops; continuation %p of type %d "
"is trying to jump from runloop %d to runloop %d]\n",
This meessage is triggered for exceptions too, which is wrong. Only
continuations that *would* possibly resume this runloop are a problem (i.e.
the 1) case above).
The TODO case I added to t/pmc/exception.t gives the following result:
not ok 34 - pushaction: error while handling error # TODO runloop shenanigans
# Failed (TODO) test (t/pmc/exception.t at line 754)
# 'main
# at_exit, flag = 1
# [oops; continuation 0x81d7128 of type 25 is trying to jump from runloop 2 to runloop 1]
# in outer handler
# in outer handler
# '
# doesn't match '/^main
# at_exit, flag = 1
# No exception handler/
# '
The "in outer handler" line is printed just before the ".end" in the
main routine. Therefore, the fact that it is printed twice suggests to
me that it must be getting executed once in the inner runloop, and again
in the outer.
I assume you are alluding to the fact that Parrot could in fact be
hacked to support transfers to other runloops that are still active.
Until then, it appears that the error is still appropriate for
Exception_Handler objects.
Re PDD23: It's not implementable the way it is. The main problem is
of course the already discussed continuation border of C code. E.g.
<cite> The embedding program
may choose to handle the exception and continue execution by invoking
the exception's continuation.
</cite>
But there's no way currently to safely invoke the continuation from C
code and continue at some arbitrary runloop nesting.
But these limitations of the current code. It could be handled by
something like the "Continuations and inferior runloops" POC I posted
9-Aug-06, or by the more comprehensive Cfunc.pod proposal with which you
followed up.
In fact, one reason I had for consolidating the stack-unwinding code
right now was to make it easier to reimplement actions based on
"Continuations and inferior runloops" technology. Is that premature?
Would you object if I did so?
I also dislike the stated mandantory closure-ish behavior of handlers. The
general case seems to be that the handler is a label in the context of the
handling call frame. If HLL semantics are different, the handler code can
still create and call a closure for handling the exception.
It occurs to me that we can have it both ways. Imagine (as Allison has
already suggested) that Parrot keeps a list of only handlers that are
active. Checking the handlers is just a matter of running through the
list and invoking each in turn [1]. Per PDD23, each handler would be a
closure, which would decide whether or not to invoke an associated
continuation. But we could also put continuations on the handler list
as well. As long as the calling conventions are compatible, this would
allow Parrot to skip the middleman at no extra cost.
Better still, it provides full backward compatibility. The existing
C<push_eh> op that takes a label could create an C<Error_Handler>
continuation just as before, so the existing syntax would continue to
have the same semantics. A new C<push_eh> op that takes a PMC could
provide either behavior, depending on whether it got a continuation or a
closure/sub.
And I don't get the rationale of:
<cite>
3 Invoke the handler (note: this is still in the thrower's
dynamic context).
</cite>
To me it seems that this should always be the exception handler's context.
In either case, it is easy for the EH to find things out about the
dynamic context in which it was created. It is [currently] impossible
for it to find out about the thrower's context unless it is run there.
That could be fixed, of course, but would require more context
switching.
Just in case it is less than clear, PDD23 splits the existing notion
of "handler" into two: a closure that is invoked in the C<throw>
context, and the cleanup code in the sub that did the C<push_eh> that
might be invoked if the closure decides to return to it. So only part
of the existing handler semantics is being "relocated" into the dynamic
context of the thrower (and the simplest part at that). The bulk of the
CATCH block body stays with the containing block, lexically and
dynamically, as it ought.
It is probably true that Perl 6 error handling as currently defined
could be implemented either way. But, FWIW, Common Lisp has additional
features that require handlers to be invoked in the dynamic scope of the
error:
1. Code nearer the point where the error is signalled is allowed to
bind dynamic variables that can affect the operation of a handler, maybe
temporarily disabling it. The spec stipulates that this sort of thing
must work.
2. CL has a 'restart' feature that depends on handlers that run in
the error context. For example:
(defun compile-foo-without-warnings ()
(handler-bind ((warning #'(lambda (condition)
(declare (ignore condition))
(invoke-restart 'muffle-warning))))
(compile 'foo)))
Within the scope of the handler-bind (which is just the call to
compile), the warning condition is bound to a handler that invokes a
restart which suppresses the printing of the warning. The
muffle-warning restart is typically bound dynamically within the warn
function itself, which signals the condition. So the handler has to run
in the error context in order to see the restart it wants.
3. Effective interactive debugging depends on not unwinding the
stack prematurely.
Admittedly, Parrot has the advantage of being able to create a
continuation that would return to the error context. But using such a
continuation to support CL error handling would seem to require some
additional mechanism to install the continuation's dynamic context
temporarily without actually invoking the continuation. That sounds
messy.
So I certainly hope we can all have our cake and eat it, too.
-- Bob
[1] Ignoring the matter of keeping the handler out of its own dynamic
context when invoked, of course.
Larry Wall
: It is probably true that Perl 6 error handling as currently defined
: could be implemented either way.
Nope, S04 specifically sez:
A C<CATCH> block sees the lexical scope in which it was defined, but
its caller is the dynamic location that threw the exception. That is,
the stack is not unwound until some exception handler chooses to
unwind it by "handling" the exception in question.
So we need the Common Lisp semantics as well.
Larry
Matt Diephouse
> From: Leopold Toetsch <l...@toetsch.at>
> Date: Mon, 18 Sep 2006 11:53:36 +0200
>
> Am Montag, 18. September 2006 03:56 schrieb Bob Rogers:
> > The attached patch consolidates most of the existing stack-unwinding
> > code into Continuation:invoke; previously, RetContinuation:invoke and
> > find_exception_handler also did stack-unwinding, and none of the three
> > did it quite the same way.
>
> Very good.
>
> Thanks.
Unfortunately, this patch breaks Tcl. There seems to be some bug with
exceptions.
Here's the Tcl used for this example:
proc test {} {uplevel #0 {append}}
test
[uplevel] executes its argument in another scope. In this case, it's
executing [append] in the global scope. This [append] call will throw
an exception because it hasn't passed enough arguments. [uplevel]
catches the exception so it can restore the call stack and then
rethrows the exception.
The code for this can be found in
languages/tcl/runtime/builtin/uplevel.pir (the actual catch happens on
line 68). The .catch() and .rethrow() macros are defined in
languages/tcl/src/macros.pir.
What's actually happening when I run this code is that the [uplevel]
code restores the call stack and rethrows the exception... and then
somehow catches it again (this is the bug), which causes a
ResizablePMCArray "can't pop from empty array" error.
I tried to write up a small test case demonstrating what the problem
was, but none of my guesses of what's causing it were correct. I hope
I've given you enough information to fix it. If I haven't, let me know
what else I can provide.
Thanks,
--
Matt Diephouse
http://matt.diephouse.com
Bob Rogers
Date: Sat, 23 Sep 2006 16:56:44 -0400
Unfortunately, this patch breaks Tcl. There seems to be some bug with
exceptions.
Here's the Tcl used for this example:
proc test {} {uplevel #0 {append}}
test
Hmm. I seem to have broken rethrow . . . or maybe not.
I tried to write up a small test case demonstrating what the problem
was, but none of my guesses of what's causing it were correct. I hope
I've given you enough information to fix it. If I haven't, let me know
what else I can provide.
I can reproduce it easily, and I agree with your analysis, but my
attempts to isolate the problem are failing, too. Stay tuned.
-- Bob
Bob Rogers
Date: Sat, 23 Sep 2006 17:43:28 -0400
From: "Matt Diephouse" <mdd...@gmail.com>
Date: Sat, 23 Sep 2006 16:56:44 -0400
Unfortunately, this patch breaks Tcl. There seems to be some bug with
exceptions.
Here's the Tcl used for this example:
proc test {} {uplevel #0 {append}}
test
Hmm. I seem to have broken rethrow . . . or maybe not.
Try the attached patch. If it works, then we have a problem, because
here's the original comment (which I deleted) that went with this line
of code:
/*
* During interpreter creation there is an initial context
* and the context of :main, created by runops_fromc_args
* Therefore, it seems, we have the main context twice
* and an exception handler in main can catch the same
* exception twich e.g. after rethrow
*
* The same problem can arise after a tailcall.
*
* So invalidate entry_type.
*/
But I can't see that either of these conditions could possibly apply
here. So we have a mystery. Possibly this was hiding some other bug,
which it would be better to identify and fix instead.
Leo?
-- Bob
Matt Diephouse
>
> Try the attached patch. If it works, then we have a problem, because
> here's the original comment (which I deleted) that went with this line
> of code:
>
> /*
> * During interpreter creation there is an initial context
> * and the context of :main, created by runops_fromc_args
> * Therefore, it seems, we have the main context twice
> * and an exception handler in main can catch the same
> * exception twich e.g. after rethrow
> *
> * The same problem can arise after a tailcall.
> *
> * So invalidate entry_type.
> */
>
> But I can't see that either of these conditions could possibly apply
> here. So we have a mystery. Possibly this was hiding some other bug,
> which it would be better to identify and fix instead.
>
> Leo?
>
That *does* work. I haven't applied it because it's not
necessarily urgent that Tcl work in trunk. I'm okay with
waiting a couple days to see if an actual fix can be found - instead
of merely using a workaround. You can feel free to apply it yourself,
of course.
Bob Rogers
Date: Sat, 23 Sep 2006 20:21:32 -0400
Bob Rogers <rogers...@rgrjr.dyndns.org> wrote:
> Try the attached patch . . .
That *does* work. I haven't applied it because it's not
necessarily urgent that Tcl work in trunk. I'm okay with
waiting a couple days to see if an actual fix can be found - instead
of merely using a workaround. You can feel free to apply it yourself,
of course.
I have found the real problem, but I'm glad you tested my patch, because
it gives me confidence that the real fix will work for you. So I've
committed it as r14697; please let me know how it goes.
-- Bob
Matt Diephouse
That works great. Thanks!