MySQL Bugs: #43560: client crashes in mysql_stmt_execute/mysql_stmt

Bug #43560	client crashes in mysql_stmt_execute/mysql_stmt_close after connection loss
Submitted:	11 Mar 13:46	Modified:	4 Aug 19:25
Reporter:	Axel Schwenke
Status:	Closed
Category:	C API	Severity:	S2 (Serious)
Version:	4.1, 5.0, 5.1, 6.0	OS:	Linux
Assigned to:	Staale Smedseng	Target Version:	5.1+
Triage:	Triaged: D2 (Serious)

Description:
If the client loses the connection to the MySQL server after mysql_stmt_prepare() then
the first call to mysql_stmt_execute() will return an error (as expected) but consecutive
calls to mysql_stmt_execute() or mysql_stmt_close() will crash the client. The expected
behavior would be error for mysql_stmt_execute() and success for mysql_stmt_close()

stack trace on 6.0:
#0  net_clear (net=0x5030e0, clear_buffer=<value optimized out>) at net.c:230
#1  0x00002b9ac77349fe in cli_stmt_execute (stmt=0x51ca60) at libmysql.c:2161
#2  0x00002b9ac77342cd in mysql_stmt_execute (stmt=0x51ca60) at libmysql.c:2578
#3  0x000000000040118e in main () at 34651.c:93

on 5.0:
#0  0x00002ae34765e3f1 in net_clear (net=0x503138) at net.c:294
#1  0x00002ae34760f30e in cli_stmt_execute (stmt=0x519a38) at libmysql.c:2495
#2  0x00002ae34760fcbf in mysql_stmt_execute (stmt=0x519a38) at libmysql.c:2842
#3  0x0000000000401191 in main () at 34651.c:93

(mysql_stmt_close() crashes in net_clear() too)

If automatic reconnect is enabled, the client will not crash. Consecutive calls to
mysql_stmt_execute() fail as expected, but mysql_stmt_errno() returns 0 - should be 2013
"Lost connection to MySQL server during query" or 2006 "MySQL server has gone away".

How to repeat:
Use the attached test program and compile it twice - with reconnect enabled and
disabled:

gcc -o test1 -DRECONNECT=0 `mysql_config --cflags --libs` 34651.c
gcc -o test2 -DRECONNECT=1 `mysql_config --cflags --libs` 34651.c

then run the test programs and stop/start the MySQL server as requested:

./test1
OK: mysql= mysql_init(NULL)
OK: mysql_options(mysql, MYSQL_OPT_RECONNECT, &reconnect)
OK: mysql_real_connect(mysql, "127.0.0.1", NULL, NULL, "test", 0, NULL, 0)
OK: mysql_query(mysql, DROP)
OK: mysql_query(mysql, CREATE)
OK: stmt= mysql_stmt_init(mysql)
OK: mysql_stmt_prepare(stmt, INSERT, strlen(INSERT))
OK: mysql_stmt_bind_param(stmt, &bind)
OK: mysql_stmt_execute(stmt)
OK: mysql_stmt_execute(stmt)
please restart MySQL server and press <return>

Error: 0 != mysql_stmt_execute(stmt)
MySQL error: 2006 - MySQL server has gone away
Segmentation fault

./test2
OK: mysql= mysql_init(NULL)
OK: mysql_options(mysql, MYSQL_OPT_RECONNECT, &reconnect)
OK: mysql_real_connect(mysql, "127.0.0.1", NULL, NULL, "test", 0, NULL, 0)
OK: mysql_query(mysql, DROP)
OK: mysql_query(mysql, CREATE)
OK: stmt= mysql_stmt_init(mysql)
OK: mysql_stmt_prepare(stmt, INSERT, strlen(INSERT))
OK: mysql_stmt_bind_param(stmt, &bind)
OK: mysql_stmt_execute(stmt)
OK: mysql_stmt_execute(stmt)
please restart MySQL server and press <return>

Error: 0 != mysql_stmt_execute(stmt)
MySQL error: 0 - Lost connection to MySQL server during query
Error: 0 != mysql_stmt_execute(stmt)
MySQL error: 2013 - Lost connection to MySQL server during query
OK: mysql_stmt_close(stmt)
OK: mysql_query(mysql, DROP)
complete

Note: the 5.0 client correctly shows mysql_stmt_errno==2013 for all but the first call to
mysql_stmt_execute(). The 5.1 and 6.0 client always return mysql_stmt_errno==0

Suggested fix:
mysql_stmt_execute() and mysql_stmt_close() must not call net_clear(stmt->mysql->net)
without testing if the connection is good.

mysql_stmt_errno() should return the correct error number if the connection was lost.

test program

Attachment: 34651.c (text/x-csrc), 2.64 KiB.

Obviously the code changed after 4.1. The following stack trace is for 4.1-bk and it is
indeed quite instructive: stmt->mysql->net->vio is already zeroed out after losing the
connection, but used in call to vio_blocking()

Program received signal SIGSEGV, Segmentation fault.
0x00002b3974dfb516 in vio_blocking (vio=0x0, set_blocking_mode=0 '\0', 
    old_mode=0x7fff35e263f7 "") at viosocket.c:81
81      viosocket.c: No such file or directory.
        in viosocket.c
(gdb) bt
#0  0x00002b3974dfb516 in vio_blocking (vio=0x0, set_blocking_mode=0 '\0', 
    old_mode=0x7fff35e263f7 "") at viosocket.c:81
#1  0x00002b3974dfc2f9 in net_clear (net=0x504138) at net.c:203
#2  0x00002b3974dbcd23 in cli_stmt_execute (stmt=0x515958) at libmysql.c:2547
#3  0x00002b3974dbd36b in mysql_stmt_execute (stmt=0x515958) at libmysql.c:2811
#4  0x0000000000400f4c in main () at 34651for41.c:90

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/74050

2884 Staale Smedseng	2009-05-14
      Bug #43560 client crashes in mysql_stmt_execute/
      mysql_stmt_close after connection loss
      
      Added test for net->vio != 0 to avoid the SIGSEGV,
      and setting the proper error code.
      
      Also, added some code to correctly associate error 
      state with the statement being executed. 
      This is due to the fact that mysql_reconnect() only 
      relates to statements if auto-reconnect is turned on.

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/74184

2884 Staale Smedseng	2009-05-15
      Bug #43560 client crashes in mysql_stmt_execute/
      mysql_stmt_close after connection loss
      
      The invalidation of client-side prepared statments
      in the event of connection loss is now factored out
      into a new function mysql_prune_stmt_list(). This
      is now used in conjunction with the end_server()
      function, rather than after-the-fact in 
      mysql_reconnect() as was previously the case.
     @ libmysql/client_settings.h
        Declaration of new function mysql_prune_stmt_list().
     @ sql-common/client.c
        Definition and use of new function mysql_prune_stmt_list().
        Removal of previously used code in mysql_reconnect().

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/74392

2885 Staale Smedseng	2009-05-18
      Bug #43560 client crashes in mysql_stmt_execute/
      mysql_stmt_close after connection loss
      
      A test case for this bug is introduced in 
      main.mysql_client_test. A corresponding debug 
      feature is introduced in the server 
      (close_conn_after_stmt_execute) to facilitate
      connection teardown. 
     @ sql/sql_prepare.cc
        The debug feature close_conn_after_stmt_execute will close 
        the current connection (socket, really) after executing the
        current prepared statement.
     @ tests/mysql_client_test.c
        A new function test_bug43560 is introduced in 
        mysql_client_test.c. It sets up a table, turns off auto 
        reconnect, activates the close_conn_after_stmt_execute, 
        and tests for the correct error states when subsequently 
        attempting to execute prepared statements.
        
        The function switches to use the TCP protocol for duration
        of the test due to the semantics of buffering with the
        AF_LOCAL protocol used by default. The connection is
        restored to AF_LOCAL upon termination of the test case. An
        extra parameter to client_connect() is added to facilitate
        the specification of protocol types.

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/74520

2888 Staale Smedseng	2009-05-19
      Bug #43560 client crashes in mysql_stmt_execute/
      mysql_stmt_close after connection loss
      
      In the event of an unexpected disconnect from the server side
      (e.g., due to a server kill and restart), a C API client may
      experience a SIGSEGV in subsequent calls to functions related
      to prepared statements in the client library.
            
      Solution: The invalidation of client-side prepared statements
      in the event of connection loss is factored out into a new
      function mysql_prune_stmt_list(). The invalidation now happens
      in conjunction with the end_server() function, rather than
      after-the-fact in mysql_reconnect() as was previously the
      case.
      
      A test case for this bug is introduced in MTR test
      main.mysql_client_test. A corresponding debug feature is
      introduced in the server (close_conn_after_stmt_execute) to
      facilitate connection teardown.
     @ libmysql/client_settings.h
        Declaration of new function mysql_prune_stmt_list().
     @ sql-common/client.c
        Definition and use of new function mysql_prune_stmt_list().
        Removal of previously used code in mysql_reconnect().
     @ sql/sql_prepare.cc
        The debug feature close_conn_after_stmt_execute will close the
        current connection after executing the current prepared
        statement.
     @ tests/mysql_client_test.c
        A new function test_bug43560() is introduced in
        mysql_client_test.c. It sets up a table, turns off auto
        reconnect, activates the close_conn_after_stmt_execute, and
        tests for the correct error states when subsequently
        attempting to execute prepared statements.
                
        The function switches to a separate connection using the TCP
        protocol for duration of the subtest due to the semantics of
        buffering with the AF_LOCAL protocol used by default.
        
        Functions client_connect() and client_disconnect() are
        modified to handle opening/closing of other than the default
        connection, as well as given parameters for specifying
        protocol type and auto reconnect mode.

Since mysql_prune_stmt_list() is only used within client.c, there's no reason to put the
declaration in client_settings.h. And this causes warnings because the sql/ directory has
its own version of client_settings.h

client.c:2638: warning: conflicting types for ‘mysql_prune_stmt_list’
client.c:927: warning: previous implicit declaration of ‘mysql_prune_stmt_list’ was
here

Okay to push aside from that.

Docs Team: 1) The 5.1 docs Chapter 21.10.5
(http://dev.mysql.com/doc/refman/5.1/en/c-api-prepared-statement-datatypes.html) states
that "To prepare a statement, pass the statement string to mysql_stmt_init(), which
returns a pointer to a MYSQL_STMT data structure." 

I think this could be more appropriately be expressed along the lines of "To prepare a
statement, pass the the MYSQL connection handle along with the query string to
mysql_stmt_prepare()."

2) This may already be documented, I've just not been able to find it yet: I think it
would be beneficial to state somewhere that a statement that has been prepared (with
mysql_stmt_prepare()) is invalidated in the event of connection loss
(CR_SERVER_GONE_ERROR or CR_SERVER_LOST), and will have to be re-prepared. This is due to
the fact that the server discards all information about prep stmts on the server side when
a connection is lost/terminated. This is also true even when auto reconnect is in effect,
as a reconnect basically replaces the connection with a new one.

Pushed into 5.1.36 (revid:matthias.leich@sun.com-20090520195829-2q0xhtjqt4gc8htf) (version
source revid:staale.smedseng@sun.com-20090520173437-ted3kt2goq39msbb) (merge vers: 5.1.36)
(pib:manually)

Pushed into 5.4.4-alpha (revid:alik@sun.com-20090617073019-azsawauatv99124t) (version
source revid:staale.smedseng@sun.com-20090520173437-ted3kt2goq39msbb) (merge vers:
5.4.4-alpha) (pib:manually)

Noted in 5.1.36, 5.4.4 changelogs.

If the client lost the connection to the MySQL server after
mysql_stmt_prepare(), the first call to mysql_stmt_execute() returned
an error (as expected) but consecutive calls to mysql_stmt_execute()
or mysql_stmt_close() crashed the client.

http://dev.mysql.com/doc/refman/5.1/en/auto-reconnect.html has a list of items showing
what information is lost when a connection goes down. Among them:

Prepared statements are released.

Staale, I've made the changes to 21.10.5. Thanks for the suggestion.

Noted in 5.4.2 changelog because next 5.4 version will be 5.4.2 and not 5.4.4.

Ignore previous comment about 5.4.2.

The 5.4 fix has been pushed to 5.4.2.