Regression in 18.104.22.168 with some ensure blocks being executed twice #4895
This example requires the sequel gem, as I have not been able to create a reproducible example that doesn't require it.
To make sure it isn't an OpenBSD specific issue, I also tested on Windows:
From some puts debugging I have found that an ensure block (https://github.com/jeremyevans/sequel/blob/5.3.0/lib/sequel/database/transactions.rb#L227) is being executed twice during the same method call when it should only be executed once.
This bug breaks Sequel when trying to use savepoints with JRuby 22.214.171.124.
The text was updated successfully, but these errors were encountered:
No need to setup pgsql, I'm using the mock adapter that doesn't actually do a database connection. Just checking out the sequel repository or installing the sequel gem will be enough. This should reproduce with a gem install:
NOTE: This is not actually a reproduction against master but against a local tree. I am leaving it as an illustration though since the description below is what must be happening (just not with this exact code).
Ok this is fallout from fixing #4865. Here is a reduced snippet:
begin raise ArgumentError("DDDD") ensure begin ensure puts "EERRRRR" end end
This will print out EERRRRR twice. Tracing through execution the raise occurs goes to a reasonable rescue ip (fairly positive it is correct cloned contents of both ensures). Then it goes to a throw instr because we are done executing the ensure and it needs to raise out to the next outer method.............
HOWEVER, the rescuePC stack is dirty at this point because of the original raise (the raise jumps over an exc_end_region). So the interpreter catches the raise and then consults the dirty stack which then decides to go down a copy of the inner ensure. That finishes popping the stack and we exit having did an extra tour through the inner ensure.
My commit message explains what changed and why I am reasonably confident about this fix. As an aside we basically did this new solution originally but instead of making a simple array of ints we would store it as a field on every instr (which was heavy enough we moved away from that). set on primitive array is quite a bit faster than a method call setting a field so this is no doubt quicker.