Skip to content

MDEV-39241: THD::backup_commit_lock warning dangling-pointer#4891

Open
grooverdan wants to merge 1 commit into
MariaDB:10.11from
grooverdan:MDEV-39241
Open

MDEV-39241: THD::backup_commit_lock warning dangling-pointer#4891
grooverdan wants to merge 1 commit into
MariaDB:10.11from
grooverdan:MDEV-39241

Conversation

@grooverdan
Copy link
Copy Markdown
Member

@grooverdan grooverdan commented Apr 2, 2026

The mechanisms of XA backup locks use a stack allocated mdl_request object. This is passed and stored in the THD object. Compilers complain about this behaviour because the lifetime of the THD->backup_commit_lock may exceed the lifetime of the stack variable mdl_request.

As the complier is also free to reuse the memory of mdl_request after its last reference in the code, the lifetime of the variable isn't defined after the function call to trans_xa_get_backup_lock.

As the THD has an allocation mechanism defined, and can release used memory, lets change this implementation and be explicit, and not rely on the compiler doing the right thing in the realm of undefined behaviour.

caused by MDEV-35110 / 066f920

@grooverdan grooverdan requested a review from knielsen April 2, 2026 03:48
@grooverdan grooverdan added the MariaDB Foundation Pull requests created by MariaDB Foundation label Apr 2, 2026
The mechanisms of XA backup locks use a stack allocated
mdl_request object. This is passed and stored in the THD
object. Compilers complain about this behaviour because
the lifetime of the THD->backup_commit_lock may exceed the
lifetime of the stack variable mdl_request.

As the complier is also free to reuse the memory of
mdl_request after its last reference in the code, the
lifetime of the variable isn't defined after the function
call to trans_xa_get_backup_lock.

As the THD has an allocation mechanism defined, and can
release used memory, lets change this implementation and
be explicit, and not rely on the compiler doing the right
thing in the realm of undefined behaviour.

Caused by MDEV-35110 / 066f920.
Copy link
Copy Markdown
Contributor

@svoj svoj left a comment

Choose a reason for hiding this comment

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

It is very wrong that the code stores MDL_request in general, it should store MDL_ticket instead. Could you check if it is doable to make this change?

@andrelkin
Copy link
Copy Markdown
Contributor

Dear gents,

MDEV-36025 patch and analysis visited this piece of code that the Jira ticket compiler complained.
And it did it wrong!

If we carefully check thd->backup_commit_lock value change in trans_xa_commit (the caller for next)
we find the following strict sequence of events:

  1. Allocation for MDL_request by the caller
  2. Use of its address as RHS in the caller's stack
  3. Reset/Cleanup with the same allocation's stack.

That is thd->backup_commit_lock == nullptr when execution leaves that caller's branch.

I did not not overlook anything?

@svoj
Copy link
Copy Markdown
Contributor

svoj commented May 21, 2026

@andrelkin you're right, it must be compiler bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

MariaDB Foundation Pull requests created by MariaDB Foundation

Development

Successfully merging this pull request may close these issues.

3 participants