Fix for #8077#8119
Conversation
…tion of code that uses ON DISCONNECT trigger
…ecuition of code that uses ON DISCONNECT trigger; Misc changes
|
On 5/22/24 17:53, Vlad Khorsun wrote:
***@***.**** commented on this pull request.
------------------------------------------------------------------------
In src/jrd/extds/ExtDS.cpp
<#8119 (comment)>:
> @@ -1900,6 +1905,11 @@ void Statement::close(thread_db* tdbb, bool invalidTran)
m_error = false;
m_transaction = NULL;
m_connection.releaseStatement(tdbb, this);
+ if ((!doPunt) && tdbb->tdbb_status_vector->hasData() &&
+ fb_utils::containsErrorCode(tdbb->tdbb_status_vector->getErrors(), isc_ses_reset_failed))
+ {
+ doPunt = true;
+ }
It depends on what exactly we want to be logged:
* original error |isc_exec_sql_max_call_exceeded|, or
* error in TRIGGER_DISCONNECT, or
* |isc_ses_reset_failed|, or
* something else ?
Everything was logged:
fbs3 Wed May 15 14:32:17 2024
Database: /mnt/db/tmp4test.fdb
Error at disconnect:
Reset of user session failed. Connection is shut down.
Execute statement error at attach :
335544830 : Too many recursion levels of EXECUTE STATEMENT
Data source : Firebird::localhost:/mnt/db/tmp4test.fdb
At trigger 'TRG_DETACH' line: 3, col: 5
|
|
Main problem is not what set of errors we log - what seems strange is that raising error in another place makes it invisible to logging call in a code, executing disconnect triggers. |
Try without connections pool. |
Error your mention is logged by |
Let me correct myself. When first attachment in chain run's own we need a way to know that our caller is Here is corrected patch: |
|
On 5/22/24 19:15, Vlad Khorsun wrote:
Main problem is not what set of errors we log - what seems
strange is that raising error in another place makes it
invisible to logging call in a code, executing disconnect
triggers.
Error your mention is logged by |purge_attachment()| when
|TRIGGER_DISCONNECT| is failed. But, if that trigger was failed
once (when called by |Attachment::resetSession()|) - it is not run
by |purge_attachment()| anymore.
Let me correct myself.
When first attachment in chain run's own |TRIGGER_DISCONNECT|, it have
no |ATT_resetting| flag set. Thus |Provider::releaseConnection()|
doesn't re-throws reset error (that happens in external attachment).
I.e. here
|// check if reset of external session failed during resetting of
parent (local) attachment const auto status =
tdbb->tdbb_status_vector; if (!inPool && (att->att_flags &
ATT_resetting) && status->hasData()) resetError.loadFrom(status); |
we need a way to know that our caller is |TRIGGER_DISCONNECT|.
Here is corrected patch:
gh8077-alt2.diff.txt
<https://github.com/FirebirdSQL/firebird/files/15406085/gh8077-alt2.diff.txt>
Please commit it to the branch, suppose PR to be merged after it.
|
…not stop execution of code that uses ON DISCONNECT trigger (FB 4.x+)
hvlad
added
type: bug
affect-version: 5.0.0
and removed
type: bug
affect-version: 5.0.0
labels
2 participants
Error "Too many recursion levels" does not stop execuition of code that uses ON DISCONNECT trigger