Fix CORE-5173 #9
Conversation
| // outer after (this block) | ||
| // inner after | ||
| // Because of this following assert is commentd out | ||
| //fb_assert(transaction->tra_save_point && transaction->tra_save_point->sav_number == count); |
There was a problem hiding this comment.
What is "before" and "after" here?
There was a problem hiding this comment.
These are parts of code managing savepoints that are placed before and after call of looper for exception handlers, as written in parentheses.
There was a problem hiding this comment.
I'd prefer to see the bad execution flow fixed rather than working around with a disabled assert.
There was a problem hiding this comment.
I'm not sure that recursive exceptions handling is a bad thing. It can be "as designed". In any case this is a bigger work for different ticket.
|
Added more protection as agreed in discussion in #10 |
| @@ -569,6 +569,7 @@ const StmtNode* BlockNode::execute(thread_db* tdbb, jrd_req* request, ExeState* | |||
| // The savepoint of this block will be dealt with below. | |||
| // Do this only if error handlers exist. If not - leave rollbacking to caller node | |||
| while (transaction->tra_save_point && | |||
| count < transaction->tra_save_point->sav_number && | |||
| transaction->tra_save_point->sav_next && | |||
| count < transaction->tra_save_point->sav_next->sav_number) | |||
| { | |||
There was a problem hiding this comment.
In tra.cpp, one condition is ">" and another one is ">=". Is it correct that both conditions in this patch are strict inequalities?
There was a problem hiding this comment.
Yes. In tra.cpp rolling forward must stop immediatelly before given savepoint, here - before one savepoint before the given one because there we must rollback the savepoint, here - rollback everything but the savepoint as it must be applied afterward.
Commented out assertion because execution flow in nodes is not what I expected when wrote this assertion. Added comment in code to remember that.