Avoid crashing when Statement#close is called before a Result is garb…

…age collected
brianmario · Nov 18, 2015 · 9c5031a · 9c5031a
1 parent 45ba8d9
commit 9c5031a
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 12 deletions.
diff --git a/ext/mysql2/result.c b/ext/mysql2/result.c
@@ -48,7 +48,7 @@ static rb_encoding *binaryEncoding;
 #define MYSQL2_MIN_TIME 62171150401ULL
 #endif
 
-#define GET_RESULT(obj) \
+#define GET_RESULT(self) \
   mysql2_result_wrapper *wrapper; \
   Data_Get_Struct(self, mysql2_result_wrapper, wrapper);
 
@@ -91,16 +91,18 @@ static void rb_mysql_result_free_result(mysql2_result_wrapper * wrapper) {
 
   if (wrapper->resultFreed != 1) {
     if (wrapper->stmt_wrapper) {
-      mysql_stmt_free_result(wrapper->stmt_wrapper->stmt);
-
-      /* MySQL BUG? If the statement handle was previously used, and so
-       * mysql_stmt_bind_result was called, and if that result set and bind buffers were freed,
-       * MySQL still thinks the result set buffer is available and will prefetch the
-       * first result in mysql_stmt_execute. This will corrupt or crash the program.
-       * By setting bind_result_done back to 0, we make MySQL think that a result set
-       * has never been bound to this statement handle before to prevent the prefetch.
-       */
-      wrapper->stmt_wrapper->stmt->bind_result_done = 0;
+      if (!wrapper->stmt_wrapper->closed) {
+        mysql_stmt_free_result(wrapper->stmt_wrapper->stmt);
+
+        /* MySQL BUG? If the statement handle was previously used, and so
+         * mysql_stmt_bind_result was called, and if that result set and bind buffers were freed,
+         * MySQL still thinks the result set buffer is available and will prefetch the
+         * first result in mysql_stmt_execute. This will corrupt or crash the program.
+         * By setting bind_result_done back to 0, we make MySQL think that a result set
+         * has never been bound to this statement handle before to prevent the prefetch.
+         */
+        wrapper->stmt_wrapper->stmt->bind_result_done = 0;
+      }
 
       if (wrapper->result_buffers) {
         unsigned int i;
@@ -858,6 +860,10 @@ static VALUE rb_mysql_result_each(int argc, VALUE * argv, VALUE self) {
 
   GET_RESULT(self);
 
+  if (wrapper->stmt_wrapper && wrapper->stmt_wrapper->closed) {
+    rb_raise(cMysql2Error, "Statement handle already closed");
+  }
+
   defaults = rb_iv_get(self, "@query_options");
   Check_Type(defaults, T_HASH);
   if (rb_scan_args(argc, argv, "01&", &opts, &block) == 1) {

diff --git a/ext/mysql2/statement.c b/ext/mysql2/statement.c
@@ -8,7 +8,8 @@ static VALUE intern_usec, intern_sec, intern_min, intern_hour, intern_day, inter
 #define GET_STATEMENT(self) \
   mysql_stmt_wrapper *stmt_wrapper; \
   Data_Get_Struct(self, mysql_stmt_wrapper, stmt_wrapper); \
-  if (!stmt_wrapper->stmt) { rb_raise(cMysql2Error, "Invalid statement handle"); }
+  if (!stmt_wrapper->stmt) { rb_raise(cMysql2Error, "Invalid statement handle"); } \
+  if (stmt_wrapper->closed) { rb_raise(cMysql2Error, "Statement handle already closed"); }
 
 
 static void rb_mysql_stmt_mark(void * ptr) {
@@ -105,6 +106,7 @@ VALUE rb_mysql_stmt_new(VALUE rb_client, VALUE sql) {
   {
     stmt_wrapper->client = rb_client;
     stmt_wrapper->refcount = 1;
+    stmt_wrapper->closed = 0;
     stmt_wrapper->stmt = NULL;
   }
 
@@ -461,6 +463,7 @@ static VALUE rb_mysql_stmt_affected_rows(VALUE self) {
  */
 static VALUE rb_mysql_stmt_close(VALUE self) {
   GET_STATEMENT(self);
+  stmt_wrapper->closed = 1;
   rb_thread_call_without_gvl(nogvl_stmt_close, stmt_wrapper, RUBY_UBF_IO, 0);
   return Qnil;
 }

diff --git a/ext/mysql2/statement.h b/ext/mysql2/statement.h
@@ -7,6 +7,7 @@ typedef struct {
   VALUE client;
   MYSQL_STMT *stmt;
   int refcount;
+  int closed;
 } mysql_stmt_wrapper;
 
 void init_mysql2_statement(void);