Parrot interpreters only die once #816

Closed
bdw opened this Issue Aug 20, 2012 · 6 comments

Comments

Projects
None yet
4 participants
Contributor

bdw commented Aug 20, 2012

Hi everybody,

In a branch of mod_parrot designed to ease the writing of exception handlers, I noted that the API cannot catch exceptions twice.

Let me be a little more specific about that. If I run some program, and it throws an exception and the interpreter 'dies', it returns to the api call via longjmp(), and sets some values upon the interpreter object. Based upon these values, the api call returns 0, upon which the exception is retrieved and inspected. Best example: show_last_error_and_exit.

What happens in mod_parrot is that the same process will have multiple interpreters, in our case subsequently, and hopefully one day parallel. When an exception is thrown and not caught, parrot will go to die_from_exception. In this, a static variable called already_dying is set. When it is non-zero, the function continues on to Parrot_x_jump_out, which returns nicely to the api handler. However, because no error flags are set, the api function assumes everything was alright.

What I'm thinking here is, that last assumption is probably nonsense. And the use of the static variable, thats probably nonsense to. We should just check for the last_exception being PMCNULL; if so you can jump out (we are already dying) and if not set the values.
So, what do you think?

Kind regards,
Bart

Owner

leto commented Aug 20, 2012

I agree, there seem to be some assumptions that are no longer true in your use case. If you would like to implement your changed assumptions in a branch, it will probably be received well.

@ghost ghost assigned Whiteknight Aug 22, 2012

rurban pushed a commit that referenced this issue Aug 22, 2012

[GH #816] Check if you're dying by interp->final_exception
In a branch of mod_parrot designed to ease the writing of
exception handlers, I noted that the API cannot catch exceptions twice.
On throwing an exception and the interpreter 'dies', it returns to the
api call via longjmp(), and sets some values upon the interpreter object.
Based upon these values, the api call returns 0, upon which the exception
is retrieved and inspected. Best example: show_last_error_and_exit.

We should just check for the last_exception being PMCNULL; if so you can
jump out (we are already dying) and if not set already_dying.
Member

rurban commented Aug 22, 2012

Looks good to me, without the cuddled else and whitespace changes.
Tests pass: http://smolder.parrot.org/app/projects/report_details/30691

I pushed the fixed commit to gh816-unstatic-dying-check, for whiteknight to counter check.

Do we have test case to repro the old problem?

Contributor

bdw commented Aug 22, 2012

I pushed a test case to my repo in t/src/embed/api.t

See this commit

Member

rurban commented Aug 22, 2012

I could reproduce the error and also the fix with master and gh816-unstatic-dying-check.
gh816-unstatic-dying-check is ready to be merged after the 4.7.0 release.

Contributor

bdw commented Aug 22, 2012

Awesome! That means I can erase my pull request?

rurban pushed a commit that referenced this issue Aug 23, 2012

[GH #816] Check if you're dying by interp->final_exception
In a branch of mod_parrot designed to ease the writing of
exception handlers, I noted that the API cannot catch exceptions twice.
On throwing an exception and the interpreter 'dies', it returns to the
api call via longjmp(), and sets some values upon the interpreter object.
Based upon these values, the api call returns 0, upon which the exception
is retrieved and inspected. Best example: show_last_error_and_exit.

We should just check for the last_exception being PMCNULL; if so you can
jump out (we are already dying) and if not set already_dying.

rurban pushed a commit that referenced this issue Aug 23, 2012

Member

rurban commented Aug 23, 2012

Fixed with cherry-pick

  • 2d0fc71 : [GH #816] Added test case for dying twice
  • 0642658 : [GH #816] Check if you're dying by interp->final_exception
    from gh816-unstatic-dying-check

@rurban rurban closed this Aug 23, 2012

@Whiteknight Whiteknight removed their assignment Mar 7, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment