Ticket #1631 (assigned bug)

Opened 4 years ago

Last modified 4 years ago

Exception handler throwing exception in NQP-rx causes segfault

Reported by: sorear Owned by: pmichaud
Priority: normal Milestone:
Component: nqp Version: trunk
Severity: high Keywords:
Cc: Language:
Patch status: Platform:

Description

{ pir::die(""); CATCH { pir::die(""); } }

The NQP-rx generated exception handler does not call pop_eh in the handler path, which apparently it should, according to bacek.

Change History

  Changed 4 years ago by coke

This code now segfaults in r47563

  Changed 4 years ago by coke

  • severity changed from medium to high

  Changed 4 years ago by jonathan

Fixed in r48074; needs tests.

  Changed 4 years ago by jonathan

pmichaud++ points out that while the patch does fix the problem at hand, it also breaks the imo less important case of resumable exceptions. So we need a fix that handles both before this ticket is closed.

19:48 <@pmichaud> jnthn: I'm a bit concerned about the fix for TT #1631
19:48 <@jnthn> Adding the pop_eh?
19:48 <@pmichaud> right.
19:48 <@jnthn> I don't think a CATCH should catch stuff thrown within it.
19:48 <@pmichaud> doing the pop_eh removes the handler.
19:48 <@ingy> greetings
19:48 <@jnthn> Right, that's what I intended.
19:48 <@ingy> jnthn: I agree
19:48 <@pmichaud> perhaps not, but triggering a CATCH shouldn't disable it for
                  all subsequent exceptions, either.
19:49 <@pmichaud> die "1";  die "2";  CATCH { resume }
19:49 <@jnthn> Oh. :S
19:49 -!- cosimo [~cosimo@pat-tdc.opera.com] has quit [Remote host closed the
          connection]
19:50 <@jnthn> OTOH, I'd take getting basic exceptions and throws from within
               the block over resumable exceptions, if it's the choice between
               them.
19:50 <@jnthn> (until we work out a way to do both)
19:50 <@pmichaud> we have to be able to handle both, though.
19:52 <@pmichaud> we need a way to throw exceptions such that they skip the
                  current handler.
19:53 <@pmichaud> (or the current handler otherwise ignores them)

in reply to: ↑ description   Changed 4 years ago by pmichaud

  • status changed from new to assigned

After thinking about it further, I've decided to revert r48074. The behavior before the patch was the correct one (it still is), and I'm not at all comfortable with changing it right before a supported Parrot release. Rakudo and nqp-rx will have to come up ways to handle throwing exceptions from inside of handlers such that the handler doesn't immediately catch them.

Reverted in r48081.

Pm

Note: See TracTickets for help on using tickets.