List:Commits« Previous MessageNext Message »
From:Davi Arnaut Date:May 19 2009 2:17pm
Subject:Re: bzr commit into mysql-5.1-bugteam branch (staale.smedseng:2885)
Bug#43560
View as plain text  
Hi Staale,

On 5/18/09 11:51 AM, Staale Smedseng wrote:
> #At file:///export/home/tmp/ss156133/z/43560-51g/ based on
> revid:staale.smedseng@stripped
>
>   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.

Good.

>      modified:
>        sql/sql_prepare.cc
>        tests/mysql_client_test.c
> === modified file 'sql/sql_prepare.cc'
> --- a/sql/sql_prepare.cc	2009-02-13 16:41:47 +0000
> +++ b/sql/sql_prepare.cc	2009-05-18 14:51:10 +0000
> @@ -2461,6 +2461,9 @@ void mysql_stmt_execute(THD *thd, char *
>
>     stmt->execute_loop(&expanded_query, open_cursor, packet, packet_end);
>
> +  /* close connection socket; for use with client testing (bug 43560) */
> +  DBUG_EXECUTE_IF("close_conn_after_stmt_execute", close(thd->net.vio->sd););
> +

Can't you call vio_close() here? or perhaps assert that its a socket 
based connection. I'm not sure close will be available here in every 
platform that we build. Comments?

>     DBUG_VOID_RETURN;
>
>   }
>
> === modified file 'tests/mysql_client_test.c'
> --- a/tests/mysql_client_test.c	2009-05-05 10:34:25 +0000
> +++ b/tests/mysql_client_test.c	2009-05-18 14:51:10 +0000
> @@ -273,7 +273,7 @@ mysql_simple_prepare(MYSQL *mysql_arg, c
>
>   /* Connect to the server */
>
> -static void client_connect(ulong flag)
> +static void client_connect(ulong flag, uint protocol)
>   {
>     int  rc;
>     static char query[MAX_TEST_QUERY_LENGTH];
> @@ -291,6 +291,7 @@ static void client_connect(ulong flag)
>     }
>     /* enable local infile, in non-binary builds often disabled by default */
>     mysql_options(mysql, MYSQL_OPT_LOCAL_INFILE, 0);
> +  mysql_options(mysql, MYSQL_OPT_PROTOCOL,&protocol);

OK.

>     if (!(mysql_real_connect(mysql, opt_host, opt_user,
>                              opt_password, opt_db ? opt_db:"test", opt_port,
> @@ -17712,6 +17713,88 @@ static void test_bug40365(void)
>   }
>
>
> +static void test_bug43560(void)
> +{
> +  uint         rc;
> +  MYSQL_STMT   *stmt= 0;
> +  MYSQL_BIND   bind;
> +  my_bool      is_null= 0;
> +  const uint   BUFSIZE= 256;
> +  char         buffer[BUFSIZE];
> +  my_bool      reconnect= 0;
> +  const char*  values[] = {"eins", "zwei", "drei", "viele", NULL};
> +  const char   insert_str[] = "INSERT INTO t1 (c2) VALUES (?)";
> +  unsigned long length;
> +
> +  DBUG_ENTER("test_bug43560");
> +  myheader("test_bug43560");
> +
> +  /* query the server version to make sure we only run against a debug server */
> +  if (!strstr(mysql->server_version, "debug"))
> +  {
> +    printf(stdout, "skipping test_bug43560: server not DEBUG version\n");

printf() that takes a FILE argument? perhaps you meant fprintf?

> +    return;

DBUG_VOID_RETURN;

> +  }
> +
> +  /* use TCP for this test to avoid problems with the buffer semantics of
> +     AF_UNIX, and turn of auto reconnect */
> +  client_disconnect();
> +  client_connect(0, MYSQL_PROTOCOL_TCP);
> +  mysql_options(mysql, MYSQL_OPT_RECONNECT,&reconnect);

Wouldn't it be better to have a new handle? This way we wouldn't mess 
with the default one that is going to be used for other test cases..

> +  rc= mysql_query(mysql, "DROP TABLE IF EXISTS t1");
> +  myquery(rc);
> +  rc= mysql_query(mysql,
> +    "CREATE TABLE t1 (c1 INT PRIMARY KEY AUTO_INCREMENT, c2 CHAR(10))");
> +  myquery(rc);
> +
> +  stmt= mysql_stmt_init(mysql);
> +  check_stmt(stmt);
> +  rc= mysql_stmt_prepare(stmt, insert_str, strlen(insert_str));
> +  check_execute(stmt, rc);
> +
> +  bind.buffer_type= MYSQL_TYPE_STRING;
> +  bind.buffer_length= BUFSIZE;
> +  bind.buffer= buffer;
> +  bind.is_null=&is_null;
> +  bind.length=&length;
> +  rc= mysql_stmt_bind_param(stmt,&bind);
> +  check_execute(stmt, rc);
> +
> +  /* first execute; should succeed */
> +  strncpy(buffer, values[0], BUFSIZE);
> +  length= strlen(buffer);
> +  rc= mysql_stmt_execute(stmt);
> +  check_execute(stmt, rc);
> +
> +  /* closes the server-side socket after execution of prep statement */
> +  rc= mysql_query(mysql,"SET SESSION debug='+d,close_conn_after_stmt_execute'");
> +  myquery(rc);
> +
> +  /* second execute; should fail due to socket closed during execution */
> +  strncpy(buffer, values[1], BUFSIZE);
> +  length= strlen(buffer);
> +  rc= mysql_stmt_execute(stmt);
> +  DIE_UNLESS(rc&&  mysql_stmt_errno(stmt) == CR_SERVER_LOST);
> +
> +  /* third execute; should fail (connection already closed) or SIGSEGV with an
> +     unpatched libmysql */
> +  strncpy(buffer, values[2], BUFSIZE);
> +  length= strlen(buffer);
> +  rc= mysql_stmt_execute(stmt);
> +  DIE_UNLESS(rc&&  mysql_stmt_errno(stmt) == CR_SERVER_LOST);
> +
> +  /* clean up and turn off the debug functionality in server */
> +  client_disconnect();
> +  client_connect(0, MYSQL_PROTOCOL_DEFAULT);
> +  rc= mysql_query(mysql,"DROP TABLE IF EXISTS t1");
> +  myquery(rc);
> +  rc= mysql_query(mysql,"SET SESSION debug='-d,close_conn_after_stmt_execute'");
> +  myquery(rc);

Clean up wouldn't be needed with a new handle.

Regards,

-- Davi Arnaut
Thread
bzr commit into mysql-5.1-bugteam branch (staale.smedseng:2885) Bug#43560Staale Smedseng18 May
  • Re: bzr commit into mysql-5.1-bugteam branch (staale.smedseng:2885)Bug#43560Davi Arnaut19 May
    • Re: bzr commit into mysql-5.1-bugteam branch (staale.smedseng:2885)Bug#43560Staale Smedseng19 May