Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix CORE-5252 #29

Merged
merged 2 commits into from
Jun 29, 2016
Merged

Fix CORE-5252 #29

merged 2 commits into from
Jun 29, 2016

Conversation

aafemt
Copy link
Contributor

@aafemt aafemt commented May 26, 2016

No description provided.

@@ -3579,6 +3585,15 @@ void jrd_tra::rollbackToSavepoint(thread_db* tdbb, SavNumber number)
*
**************************************/
{
#ifdef SAVEPOINT_DEBUG
printf("Request to rollback savepoint %" QUADFORMAT "d. Stack: ", number);
for (Savepoint* itr = tra_save_point; itr; itr = itr->getNext())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, don't use printf for debug purposes. Use common way and define appropriate (for ex SAV_trace) routine

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On 05/26/2016 05:22 PM, Dimitry Sibiryakov wrote:

@@ -3579,6 +3585,15 @@ void jrd_tra::rollbackToSavepoint(thread_db* tdbb, SavNumber number)
*
**************************************/
{
+#ifdef SAVEPOINT_DEBUG

  • printf("Request to rollback savepoint %" QUADFORMAT "d. Stack: ", number);
  • for (Savepoint* itr = tra_save_point; itr; itr = itr->getNext())
    Ok.

And it should go to stderr, not stdout.
Only stderr (2) remains opened on posix when -d switch is used, stdout
(1) can become network socket or something else.

}

// Release stored looper savepoints
Savepoint::destroy(request->req_savepoints);
Copy link
Contributor Author

@aafemt aafemt Jun 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line causes engine to crash on test bugs.core_0214 as if savepoint was released from non-existing pool, but I cannot understand how it can be: pointers to transaction and pool are valid.
Changing it to Savepoint::mergeStacks(request->req_transaction->tra_save_free, request->req_savepoints); helps, but I'm afraid that it only put problem off, not fixing it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On 06/14/2016 01:13 PM, Dimitry Sibiryakov wrote:

     return;
  • }
  • // Release stored looper savepoints
  • Savepoint::destroy(request->req_savepoints);
    This line causes engine to crash on test bugs.core_0214 as if savepoint was released from non-existing pool, but I cannot understand how it can be: pointers to transaction and pool are valid.

Always good idea to provide stack trace in such cases.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On 06/14/2016 01:58 PM, Dimitry Sibiryakov wrote:

engine13.dll!Firebird::MemPool::release(void * object, bool flagDecr) Line 2125 C++

at this frame please print *block and *pool

Copy link
Contributor Author

@aafemt aafemt Jun 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I see that block's pool doesn't match savepoint's transaction pool. But savepoint's transaction match request transaction.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this block 'pool' pointer is invalid, 'next' pointer is valid, but code handle it as a pointer to pool, that's a reason for crash. Perhaps, single savepoints are not supposed to be freed at all, they will die at once with transaction pool...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On 06/14/2016 04:47 PM, Dimitry Sibiryakov wrote:

     return;
  • }
  • // Release stored looper savepoints
  • Savepoint::destroy(request->req_savepoints);
    For this block 'pool' pointer is invalid, 'next' pointer is valid, but code handle it as a pointer to pool, that's a reason for crash.

That means that code tries to free some block once again.

Perhaps, single savepoints are not supposed to be freed at all, they will die at once with transaction pool...


You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
https://github.com/FirebirdSQL/firebird/pull/29/files/057fc3a9ee0a851a39de07ad6497b28ac48ce931#r66972581

@aafemt
Copy link
Contributor Author

aafemt commented Jun 14, 2016

Finally I found the reason of crash.

@dyemanov dyemanov self-assigned this Jun 17, 2016
@dyemanov dyemanov merged commit 53630eb into FirebirdSQL:master Jun 29, 2016
@aafemt aafemt deleted the CORE-5252 branch June 30, 2016 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants