Bug #77689 mysql_execute_command SQLCOM_UNLOCK_TABLES redundant trans_check_state check?
Submitted: 11 Jul 2015 14:27 Modified: 27 Jan 2016 11:36
Reporter: Laurynas Biveinis (OCA) Email Updates:
Status: Open Impact on me:
Category:MySQL Server: Locking Severity:S3 (Non-critical)
Version:5.6,5.7 OS:Any
Assigned to: CPU Architecture:Any
Tags: cleanup

[11 Jul 2015 14:27] Laurynas Biveinis
mysql_execute_command(THD *thd)
    if (thd->variables.option_bits & OPTION_TABLE_LOCK)
        Can we commit safely? If not, return to avoid releasing
        transactional metadata locks.
      if (trans_check_state(thd))

trans_check_state returns TRUE iff we are either in a SP, either in an XA transaction. But SP cannot contain UNLOCK TABLES command, and XA cannot have OPTION_TABLE_LOCK at this point (it cannot be active at XA START nor you can issue a command that would set it inside XA).

So, is trans_check_state here dead code? It was added by b6c3c7978e89dc097889bd38d04fed58b991a828, and it is not clear why.

Obviously this is a very minor bug if confirmed, this is more of me asking if my code understanding here is correct.

How to repeat:
Code analysis

Suggested fix:
Remove trans_check_state call
[11 Jul 2015 14:27] Laurynas Biveinis
commit b6c3c7978e89dc097889bd38d04fed58b991a828
Author: Jon Olav Hauglid <jon.hauglid@oracle.com>
Date:   Wed Apr 10 13:44:46 2013 +0200

    If statements causing implicit commit are executed with an XA
    transaction in ACTIVE state, "The command cannot be executed
    when global transaction is in the ACTIVE state" error is
    reported. This error causes the implicit commit to fail (as it should).
    The problem was that if the implicit commit at the start of a
    statement execution failed, we also tried to commit at the end
    of the statement. This commit would fail for the same reason.
    However for this commit we assumed that the transaction
    was either successfully committed or rolled back and therefore
    released transactional metadata locks.
    This meant that the objects accessed earlier in the XA transaction
    no longer were protected and could be e.g. dropped by other
    connections. This again could lead to assertions, valgrind warnings
    and various other problems when the XA transaction later tried
    to commit.
    This patch fixes the problem by aborting the statement if the
    implicit commit at the beginning of the statement execution fails.
    This means that the release of transactional locks at the end
    of execution of statements causing implicit commit, will be skipped.
[27 Jan 2016 11:36] Laurynas Biveinis
See also remotely related bug 80174