MySQL Bugs: #58136: Crash in vio_close at concurrent disconnect and KILL

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	CPU Architecture:	Any

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.

Patch to reproduce problem

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

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.

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.

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.

Queued to mysql-5.5-bugteam and up.

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)

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)

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.