Bug #58136 Crash in vio_close at concurrent disconnect and KILL
Submitted: 11 Nov 2010 13:05 Modified: 11 Jan 2011 16:22
Reporter: Mikael Ronström Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: Locking Severity:S1 (Critical)
Version:MySQL 5.5 OS:Any
Assigned to: Davi Arnaut
Triage: Triaged: D1 (Critical)

[11 Nov 2010 13:05] Mikael Ronström
Description:
close_connection is protected by the wrong mutex and also
misses a call to thd->clear_active_vio. It should be
protected by thd->LOCK_thd_data.

How to repeat:
It requires some advanced test case where a combination of
3 DEBUG_SYNC points + a DBUG_EXECUTE_IF with a 2 sec sleep
as the action.

Will upload a patch with test case + source changes to reproduce bug.

Suggested fix:
Use thd->LOCK_thd_data as protection of close_connection,
could also add call to thd->clear_active_vio in there.
[11 Nov 2010 13:17] Mikael Ronström
Patch to reproduce problem

Attachment: kill_bug.patch (text/x-diff), 2.91 KiB.

[13 Dec 2010 11:12] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/126613

3185 Davi Arnaut	2010-12-13
      Bug#58136: Crash in vio_close at concurrent disconnect and KILL
      
      The problem is a race between a session closing its vio
      (i.e. after a COM_QUIT) at the same time it is being killed by
      another thread. This could trigger a assertion in vio_close()
      as the two threads could end up closing the same vio, at the
      same time. This could happen due to the implementation of
      SIGNAL_WITH_VIO_CLOSE, which closes the vio of the thread
      being killed.
      
      The solution is to serialize the close of the Vio under
      LOCK_thd_data, which protects THD data.
     @ sql/mysqld.cc
        Drop lock parameter from close_connection, its not necessary
        any more. The newly introduced THD::disconnect method will take
        care of locking.
     @ sql/mysqld.h
        Change prototype, add a default parameter for the error code.
     @ sql/sql_class.cc
        In case SIGNAL_WITH_VIO_CLOSE is defined, the active vio is
        closed and cleared. Subsequent calls will only close the vio
        owned by the session.
[15 Dec 2010 12:28] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/126927

3196 Davi Arnaut	2010-12-15
      Bug#58136: Crash in vio_close at concurrent disconnect and KILL
      
      The problem is a race between a session closing its vio
      (i.e. after a COM_QUIT) at the same time it is being killed by
      another thread. This could trigger a assertion in vio_close()
      as the two threads could end up closing the same vio, at the
      same time. This could happen due to the implementation of
      SIGNAL_WITH_VIO_CLOSE, which closes the vio of the thread
      being killed.
      
      The solution is to serialize the close of the Vio under
      LOCK_thd_data, which protects THD data.
     @ sql/mysqld.cc
        Drop lock parameter from close_connection, its not necessary
        any more. The newly introduced THD::disconnect method will take
        care of locking.
     @ sql/mysqld.h
        Change prototype, add a default parameter for the error code.
     @ sql/sql_class.cc
        In case SIGNAL_WITH_VIO_CLOSE is defined, the active vio is
        closed and cleared. Subsequent calls will only close the vio
        owned by the session.
[15 Dec 2010 23:00] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/127008

3196 Davi Arnaut	2010-12-15
      Bug#58136: Crash in vio_close at concurrent disconnect and KILL
      
      The problem is a race between a session closing its vio
      (i.e. after a COM_QUIT) at the same time it is being killed by
      another thread. This could trigger a assertion in vio_close()
      as the two threads could end up closing the same vio, at the
      same time. This could happen due to the implementation of
      SIGNAL_WITH_VIO_CLOSE, which closes the vio of the thread
      being killed.
      
      The solution is to serialize the close of the Vio under
      LOCK_thd_data, which protects THD data.
      
      No regression test is added as this is essentially a debug
      issue and the test case would be quite convoluted as we would
      need to synchronize a session that is being killed -- which
      is a bit difficult since debug sync points code does not
      synchronize killed sessions.
     @ sql/mysqld.cc
        Drop lock parameter from close_connection, its not necessary
        any more. The newly introduced THD::disconnect method will take
        care of locking.
     @ sql/mysqld.h
        Change prototype, add a default parameter for the error code.
     @ sql/sql_class.cc
        In case SIGNAL_WITH_VIO_CLOSE is defined, the active vio is
        closed and cleared. Subsequent calls will only close the vio
        owned by the session.
[15 Dec 2010 23:20] Davi Arnaut
Queued to mysql-5.5-bugteam and up.
[17 Dec 2010 12:50] Bugs System
Pushed into mysql-5.5 5.5.9 (revid:georgi.kodinov@oracle.com-20101217124733-p1ivu6higouawv8l) (version source revid:davi.arnaut@oracle.com-20101215225921-xrma7cdmpu2jbo05) (merge vers: 5.5.8) (pib:24)
[17 Dec 2010 12:55] Bugs System
Pushed into mysql-trunk 5.6.1 (revid:georgi.kodinov@oracle.com-20101217125013-y8pb3az32rtbplc9) (version source revid:davi.arnaut@oracle.com-20101215231037-swrblumh9rj3sj5i) (merge vers: 5.6.1) (pib:24)
[11 Jan 2011 16:22] Paul Dubois
Noted in 5.5.9 changelog.

An assertion could be raised if the server was closing a session at
the same time the session was being killed by another thread.