From: Bob Rogers <[email protected]>
Date: Tue, 8 Dec 2009 21:52:18 -0500
From: Andrew Whitworth <[email protected]>
Date: Tue, 8 Dec 2009 08:08:01 -0500
On Mon, Dec 7, 2009 at 11:43 PM, Bob Rogers wrote:
> . . . Since it seems that we are stuck with inferior runloops for
> the foreseeable future, maybe half a glass would be better than
> none.
I would like to see more information from you about this idea. I can't
imagine that it's going to resolve all our problems but it sounds
quite interesting nonetheless and aligns with some ideas I've been
kicking around.
OK, I'll try to elaborate when I have more time. With any luck, I may
get a chance to hack up a PoC this weekend.
This turned out to be easier than I expected, due mostly to the new
"struct parrot_runloop_t" machinery. The first attachment contains a
test case that illustrates the problem: A sort function that is induced
to throw an error by a buggy sort. Running this in an unpatched Parrot
gives the following output:
rog...@rgr> ./parrot ./t/pmc/test_sort_eh.t
1..1
throwing
ok 1 - buggy sort on FixedPMCArray did throw
ok 2 - test_sort_eh_inner normal return
throwing
No exception handler and no message
current instr.: 'integer_cmp_fun' pc 79 (test_sort_eh.t:44)
rog...@rgr>
At first, it looks like it all worked, but the exception was thrown to
the inner runloop, so when that returns, the sort resumes execution as
if the comparison function had returned normally, and subsequently
encounters another error. (Though I don't understand why the outer
handler doesn't catch the second error; is this a bug in handler
scoping?)
Applying the patch in the second attachment and then disabling the
search loop in ExceptionHandler:invoke shows what is going on:
rog...@rgr> ./parrot test_sort_eh.t
1..1
throwing
[oops; continuation 0x80be08c of type 18 is trying to jump from runloop
12 to runloop 1]
ok 1 - buggy sort on FixedPMCArray did throw
ok 2 - test_sort_eh_inner normal return
throwing
No exception handler and no message
current instr.: 'integer_cmp_fun' pc 79 (test_sort_eh.t:44)
rog...@rgr>
Enabling the search loop gives the intended result (complete with
debugging output):
rog...@rgr> ./parrot test_sort_eh.t
1..1
throwing
[continuation 0x80be08c of type 18 is jumping from runloop 12 to
runloop 1]
ok 1 - buggy sort on FixedPMCArray did throw
ok 2 - test_sort_eh_inner normal return
rog...@rgr>
How it works, of course, is simple: Every continuation (not just those
of type ExceptionHandler) remembers the runloop that initialized it. If
we define an "active runloop" as one for which "runops" hasn't exited
yet, then all active runloops are reachable from the
interp->current_runloop linked list. Therefore, if we can't find the
right runloop for a given continuation, then it must no longer be
active. If we *can* find it, then we have it's jump point, and can just
go there.
There are a number of limitations that make this a proof-of-concept
rather than a viable patch:
1. This only works for the ExceptionHandler class; it should be
refactored so it works for all continuation classes.
2. The "oops" warning should instead throw an error. In order to
avoid looping, the unreachable continuation must have been removed as a
handler by that point; I think that has happened by then, but wouldn't
swear to it.
3. The unwinding of interp->current_runloop leaks parrot_runloop_t
structures (but see below).
4. As alluded to earlier in the thread, it would be useful to have
some mechanism for breaking the runloop chain, so that user C code that
calls into Parrot ops can insist on not being thrown through.
In addition, there is a potential optimization: The parrot_runloop_t
structures need not be malloc'ed. Since they exist only to maintain a
stack of setjmp locations, and since those locations are only valid
while the runloop is active, it would be a bug to encounter a
parrot_runloop_t after its corresponding runops invocation has exited.
Therefore, it ought to be possible to allocate these structures in the
body of runops, so that they are maintained on the C stack. However,
there are a number of new_runloop_jump_point calls that would have to be
changed, so this may not be as straightforward as it looks.
-- Bob
#! parrot
# Copyright (C) 2001-2009, Parrot Foundation.
# From: $Id: fixedpmcarray.t 42684 2009-11-21 13:40:19Z jkeenan $
=head1 NAME
t/pmc/test_sort_eh.t - Test error handling in C<FixedPMCArray> sorting
=head1 SYNOPSIS
% prove t/pmc/test_sort_eh.t
=head1 DESCRIPTION
This is involves a nonlocal exit out of an inferior runloop.
=cut
.sub main :main
.include 'test_more.pir'
plan(1)
test_sort_eh()
.end
## Sort comparison function that insists that both args be integers.
.sub integer_cmp_fun
.param pmc a
.param pmc b
$I0 = isa a, 'Integer'
unless $I0 goto oops
$I0 = isa b, 'Integer'
unless $I0 goto oops
$I0 = cmp a, b
.local pmc compares
compares = get_global "compares"
inc compares
.begin_return
.set_return $I0
.end_return
oops:
printerr "throwing\n"
$P0 = new ['Exception']
throw $P0
.end
.sub test_sort_eh
push_eh sort_err_2
test_sort_eh_inner()
ok(1, 'test_sort_eh_inner normal return')
.return ()
sort_err_2:
ok(0, 'buggy sort on FixedPMCArray threw twice')
.end
.sub test_sort_eh_inner
.local pmc compares, integer_cmp_fun
compares = new ['Integer']
compares = 0
set_global "compares", compares
integer_cmp_fun = get_global "integer_cmp_fun"
push_eh sort_err
buggy_sort_ar(integer_cmp_fun)
ok(0, 'buggy sort on FixedPMCArray did not throw')
.return ()
sort_err:
ok(1, 'buggy sort on FixedPMCArray did throw')
.return ()
.end
## This sort is buggy because it uses an integer-only comparison function to
## sort an array with a non-integer element.
.sub buggy_sort_ar
.param pmc cmp_fun :optional
.local pmc compares
compares = get_global "compares"
compares = 0
.local pmc array
array = new ['FixedPMCArray']
array = 5
array[0] = 12
array[1] = 2
array[2] = 'foo bar'
array[3] = 9
array[4] = 1
array."sort"(cmp_fun)
ok(0, 'buggy sort on FixedPMCArray did not throw')
.end
* include/parrot/call.h:
+ (struct parrot_runloop_t): Add a runloop_id member.
* src/call/ops.c:
+ (new_runloop_jump_point): Initialize jump_point->runloop_id.
* src/pmc/exceptionhandler.pmc:
+ (invoke): If we need to transfer control to a different runloop,
search the parrot_runloop_t list to find it.
Index: include/parrot/call.h
===================================================================
--- include/parrot/call.h (revision 43027)
+++ include/parrot/call.h (working copy)
@@ -28,6 +28,7 @@
struct parrot_runloop_t *prev; /* interpreter's runloop
* jump buffer stack */
opcode_t *handler_start; /* Used in exception handling */
+ int runloop_id; /* For finding the right runloop */
/* let the biggest element cross the cacheline boundary */
Parrot_jump_buff resume; /* jmp_buf */
Index: src/call/ops.c
===================================================================
--- src/call/ops.c (revision 43027)
+++ src/call/ops.c (working copy)
@@ -104,8 +104,8 @@
free_runloop_jump_point(interp);
#if RUNLOOP_TRACE
- fprintf(stderr, "[exiting loop %d, level %d]\n",
- our_runloop_id, our_runloop_level);
+ fprintf(stderr, "[exiting loop %d, level %d, to loop %d]\n",
+ our_runloop_id, our_runloop_level, old_runloop_id);
#endif
interp->current_runloop_level = our_runloop_level - 1;
@@ -145,6 +145,7 @@
jump_point = mem_allocate_typed(Parrot_runloop);
jump_point->prev = interp->current_runloop;
+ jump_point->runloop_id = interp->current_runloop_id;
interp->current_runloop = jump_point;
}
Index: src/pmc/exceptionhandler.pmc
===================================================================
--- src/pmc/exceptionhandler.pmc (revision 43027)
+++ src/pmc/exceptionhandler.pmc (working copy)
@@ -120,6 +120,7 @@
VTABLE opcode_t *invoke(void *next) {
opcode_t * const pc = PARROT_CONTINUATION(SELF)->address;
+ Parrot_Continuation_attributes * const cc = PARROT_CONTINUATION(SELF);
Parrot_continuation_check(interp, SELF);
Parrot_continuation_rewind_environment(interp, SELF);
@@ -128,6 +129,42 @@
if (INTERP->code != PARROT_CONTINUATION(SELF)->seg)
Parrot_switch_to_cs(INTERP, PARROT_CONTINUATION(SELF)->seg, 1);
+ /* Check to see if we need to transfer control to a different runloop.
*/
+ if (INTERP->current_runloop_id != cc->runloop_id
+ /* it's ok if we are exiting to "runloop 0"; there is no such
+ runloop, but the only continuation that thinks it came from
+ runloop 0 is for the return from the initial sub call. */
+ && cc->runloop_id != 0
+ /* since a RetContinuation [currently] only returns to the next
+ outer frame, exiting to the inner run loop does the right thing,
+ since it normally returns to the next outer runloop anyway. */
+ && pmc->vtable->base_type != enum_class_RetContinuation) {
+
+ /* Search for the continuation's runloop. */
+ Parrot_runloop *jump_point = INTERP->current_runloop;
+ while (jump_point->runloop_id != cc->runloop_id &&
jump_point->prev) {
+ jump_point = jump_point->prev;
+ }
+
+ if (jump_point->runloop_id == cc->runloop_id) {
+ /* The runloop is still active, so we can go there. */
+ fprintf(stderr, "[continuation %p of type %d "
+ "is jumping from runloop %d to runloop %d]\n",
+ pmc, (int) pmc->vtable->base_type,
+ INTERP->current_runloop_id, cc->runloop_id);
+ INTERP->current_runloop = jump_point; /* bug: leaks */
+ PARROT_ASSERT(jump_point->handler_start == NULL);
+ jump_point->handler_start = pc;
+ longjmp(jump_point->resume, 2);
+ }
+ /* Couldn't find the runloop, so it must not still be active. */
+ /* [this should throw an error. -- rgr, 13-Dec-09.] */
+ fprintf(stderr, "[oops; continuation %p of type %d "
+ "is trying to jump from runloop %d to runloop %d]\n",
+ pmc, (int) pmc->vtable->base_type,
+ INTERP->current_runloop_id, cc->runloop_id);
+ }
+
return pc;
}
_______________________________________________
http://lists.parrot.org/mailman/listinfo/parrot-dev
