Bug #25584097: MYSQL_STMT_CLOSE HANGS WHEN THE STATEMENT IS CANCELED WITH KILL QUERY

Two problems:

Problem1:

The client library was not handling killing a cursor producing
mysql_stmt_execute being killed.
Basically it was not copying the error from net into the prepared
statement's error area and was not resetting the state when trying to read the result
and failing. Hence, at close, the client was trying to read the rest of
the resultset and failing.

Fixed by properly handling the error condition.

Problem2:
The server side materialized cursor wasn't closed properly when a KILL was sent
while the statement was still running and did not receive all of the data from the
cursor.

Fixed by making sure the cursor is properly disposed of.

Test case added.

Change-Id: Iff8305972948c4239bead8bd08b11088289a297e

Author: gkodinov

libmysql/libmysql.cc

@@ -1760,8 +1760,12 @@ static bool execute(MYSQL_STMT *stmt, char *packet, ulong length,
         (the reset of the result set will be read in prepare_to_fetch_result).
       */
 
-      if ((pkt_len = cli_safe_read(mysql, &is_data_packet)) == packet_error)
+      if ((pkt_len = cli_safe_read(mysql, &is_data_packet)) == packet_error) {
+        set_stmt_errmsg(stmt, net);
+        mysql->status = MYSQL_STATUS_READY;
+        stmt->read_row_func = stmt_read_row_no_data;
         return true;
+      }
 
       if (is_data_packet) {
         assert(stmt->result.rows == 0);

sql/sql_prepare.cc

@@ -3467,6 +3467,15 @@ bool Prepared_statement::execute(THD *thd, String *expanded_query,
   stmt_backup.save_rlb(thd);
 
   auto execute_guard = create_scope_guard([&]() {
+    // In an error situation, cursor may have been left open, close it:
+    if (status && open_cursor) {
+      if (m_cursor == nullptr && m_cursor_result != nullptr) {
+        m_cursor = m_cursor_result->cursor();
+      }
+      if (m_cursor != nullptr && m_cursor->is_open()) {
+        close_cursor();
+      }
+    }
     /*
       Restore the current database (if changed).
 

testclients/mysql_client_test.cc

@@ -36,7 +36,10 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <sys/types.h>
+#include <condition_variable>
 #include <memory>
+#include <mutex>
+#include <thread>
 
 #include "my_byteorder.h"
 #include "my_compiler.h"
@@ -23283,6 +23286,76 @@ static void test_bug33535746() {
   myquery(rc);
 }
 
+static void test_bug25584097() {
+  DBUG_TRACE;
+  myheader("test_bug25584097");
+  int rc;
+
+  class test_bug25584097_thd {
+    unsigned long thread_id{0};
+    std::condition_variable cnd;
+    std::mutex mtx;
+
+   public:
+    void run() {
+      int rc;
+      MYSQL *lmysql;
+      MYSQL_STMT *stmt;
+      const char *sqlstmt = "select sleep(300)";
+      unsigned long ct = (unsigned long)CURSOR_TYPE_READ_ONLY;
+
+      printf("child thread start\n");
+      lmysql = mysql_client_init(nullptr);
+      DIE_UNLESS(lmysql);
+
+      if (!mysql_real_connect(lmysql, opt_host, opt_user, opt_password,
+                              current_db, opt_port, opt_unix_socket, 0)) {
+        fprintf(stderr, "Failed to connect to the database\n");
+        DIE_UNLESS(0);
+      }
+
+      {
+        std::unique_lock lk(mtx);
+        thread_id = mysql_thread_id(lmysql);
+      }
+      stmt = mysql_stmt_init(lmysql);
+      DIE_UNLESS(stmt != nullptr);
+      rc = mysql_stmt_prepare(stmt, sqlstmt, (unsigned long)strlen(sqlstmt));
+      DIE_UNLESS(rc == 0);
+      rc = mysql_stmt_attr_set(stmt, STMT_ATTR_CURSOR_TYPE, (const void *)&ct);
+      DIE_UNLESS(rc == 0);
+      printf("child thread ready to exec\n");
+      cnd.notify_one();
+      rc = mysql_stmt_execute(stmt);
+      DIE_UNLESS(rc != 0);
+      printf("child thread cleaning up\n");
+      rc = mysql_stmt_close(stmt);
+      DIE_UNLESS(rc == 0);
+      mysql_close(lmysql);
+      printf("child thread ending\n");
+    }
+
+    unsigned long wait_to_kill() {
+      std::unique_lock lk(mtx);
+      cnd.wait(lk, [this] { return thread_id != 0; });
+      return thread_id;
+    }
+  } foo;
+
+  std::thread thd(&test_bug25584097_thd::run, &foo);
+  printf("Waiting for the child thread\n");
+  unsigned long thd_to_kill = foo.wait_to_kill();
+  sleep(2);
+
+  printf("Killing the child thread\n");
+  char cmd[50];
+  sprintf(cmd, "KILL %lu", thd_to_kill);
+  rc = mysql_query(mysql, cmd);
+  myquery(rc);
+  printf("Wating for the child thread to finish\n");
+  thd.join();
+}
+
 static struct my_tests_st my_tests[] = {
     {"test_bug5194", test_bug5194},
     {"disable_query_logs", disable_query_logs},
@@ -23596,6 +23669,7 @@ static struct my_tests_st my_tests[] = {
     {"test_wl13075", test_wl13075},
     {"test_bug34007830", test_bug34007830},
     {"test_bug33535746", test_bug33535746},
+    {"test_bug25584097", test_bug25584097},
     {nullptr, nullptr}};
 
 static struct my_tests_st *get_my_tests() { return my_tests; }