Fix for order by crash, release dangling cursor when unlocking table.

mysql-test/suite/oqgraph/regression_1133093.result

@@ -0,0 +1,44 @@
+DROP TABLE IF EXISTS graph_base;
+DROP TABLE IF EXISTS graph;
+CREATE TABLE graph_base (
+from_id INT UNSIGNED NOT NULL,
+to_id INT UNSIGNED NOT NULL,
+another_id INT UNSIGNED NOT NULL DEFAULT 1,
+w DOUBLE NOT NULL DEFAULT 1,
+PRIMARY KEY (from_id,to_id),
+INDEX (to_id)
+) ENGINE=MyISAM;
+CREATE TABLE graph (
+latch   VARCHAR(32) NULL,
+origid  BIGINT    UNSIGNED NULL,
+destid  BIGINT    UNSIGNED NULL,
+weight  DOUBLE    NULL,
+seq     BIGINT    UNSIGNED NULL,
+linkid  BIGINT    UNSIGNED NULL,
+KEY (latch, origid, destid) USING HASH,
+KEY (latch, destid, origid) USING HASH
+) ENGINE=OQGRAPH DATA_TABLE='graph_base' ORIGID='from_id', DESTID='to_id', WEIGHT='w';
+INSERT INTO graph_base(from_id, to_id) VALUES (1,2), (2,1);
+INSERT INTO graph_base(from_id, to_id) VALUES (1,3), (3,1);
+INSERT INTO graph_base(from_id, to_id) VALUES (1,4), (4,1);
+INSERT INTO graph_base(from_id, to_id) VALUES (3,4), (4,3);
+SELECT * from graph;
+latch	origid	destid	weight	seq	linkid
+NULL	1	2	1	NULL	NULL
+NULL	2	1	1	NULL	NULL
+NULL	1	3	1	NULL	NULL
+NULL	3	1	1	NULL	NULL
+NULL	1	4	1	NULL	NULL
+NULL	4	1	1	NULL	NULL
+NULL	3	4	1	NULL	NULL
+NULL	4	3	1	NULL	NULL
+SELECT * FROM graph WHERE latch='1' and destid=2 and origid=1;
+latch	origid	destid	weight	seq	linkid
+1	1	2	NULL	0	1
+1	1	2	1	1	2
+SELECT * FROM graph WHERE latch='1' and destid=2 and origid=1 order by seq;
+latch	origid	destid	weight	seq	linkid
+1	1	2	NULL	0	1
+1	1	2	1	1	2
+DROP TABLE graph;
+DROP TABLE graph_base;

mysql-test/suite/oqgraph/regression_1133093.test

@@ -34,8 +34,8 @@ INSERT INTO graph_base(from_id, to_id) VALUES (1,3), (3,1);
 INSERT INTO graph_base(from_id, to_id) VALUES (1,4), (4,1);
 INSERT INTO graph_base(from_id, to_id) VALUES (3,4), (4,3);
 
-#SELECT * from graph;
+SELECT * from graph;
-#SELECT * FROM graph WHERE latch='1' and destid=2 and origid=1;
+SELECT * FROM graph WHERE latch='1' and destid=2 and origid=1;
 SELECT * FROM graph WHERE latch='1' and destid=2 and origid=1 order by seq;
 
 DROP TABLE graph;

storage/oqgraph/graphcore.cc

@@ -147,6 +147,17 @@ namespace open_query
       HAVE_EDGE = 4,
     };
 
+    // Force assignment operator, so we can trace through in the debugger
+    inline reference& operator=(const reference& ref) 
+    {    
+      m_flags = ref.m_flags;
+      m_sequence = ref.m_sequence;
+      m_vertex = ref.m_vertex;
+      m_edge = ref.m_edge;
+      m_weight = ref.m_weight;
+      return *this;
+    }
+
     inline reference()
       : m_flags(0), m_sequence(0),
         m_vertex(graph_traits<Graph>::null_vertex()),
@@ -162,7 +173,8 @@ namespace open_query
     inline reference(int s, Vertex v, const optional<Edge> &e,
                      const optional<EdgeWeight> &w)
       : m_flags(HAVE_SEQUENCE | (w ? HAVE_WEIGHT : 0) | (e ? HAVE_EDGE : 0)),
-        m_sequence(s), m_vertex(v)
+        m_sequence(s), m_vertex(v),
+        m_edge(), m_weight(0)
     {
       if (w) m_weight= *w;
       if (e) m_edge= *e;
@@ -685,6 +697,15 @@ namespace open_query
     if (retainedLatch) { lastRetainedLatch = strdup(retainedLatch); }
   }
 
+  // Because otherwise things can happen and we havent freed a resource since the end of the last query...
+  void oqgraph::release_cursor() throw() {
+    if (share->g._cursor) {
+      delete share->g._cursor;
+    }
+    delete cursor; cursor= 0;
+    row_info= empty_row;
+  }
+
 
   int oqgraph::search(int *latch, VertexID *orig_id, VertexID *dest_id) throw()
   {
@@ -947,7 +968,15 @@ namespace open_query
     if (cursor)
       cursor->current(ref);
     else
-      ref = reference(); // avoid assignment operator because the intrusive_ptr swaps for unknown reasons, which means if ref is uninitialised it segfaults
+      // Beware: internally this eventually causes a swap by intrusive_ptr, so ref must be initialised to sane on all cases
+      ref = reference(); 
+  }
+
+  void oqgraph::init_row_ref(void *ref_ptr) throw()
+  {
+    // Placement new will cause a constructor to be called avoiding the assignment operator of intrusive_ptr
+    // This doesnt allocate any memory, assumes ref_ptr is the correct size(!)
+    new (ref_ptr) reference(); 
   }
 
   int oqgraph::random(bool scan) throw()

storage/oqgraph/graphcore.h

@@ -121,6 +121,7 @@ namespace open_query
     int fetch_row(row&) throw();
     int fetch_row(row&, const void*) throw();
     void row_ref(void*) throw();
+    void init_row_ref(void*) throw();
 
     static oqgraph* create(oqgraph_share*) throw();
     static oqgraph_share *create(TABLE*,Field*,Field*,Field*) throw();
@@ -128,6 +129,8 @@ namespace open_query
     static void free(oqgraph*) throw();
     static void free(oqgraph_share*) throw();
 
+    void release_cursor() throw();
+
     static const size_t sizeof_ref;
   private:    
     char *lastRetainedLatch;

storage/oqgraph/ha_oqgraph.cc

@@ -795,7 +795,8 @@ int ha_oqgraph::index_read(byte * buf, const byte * key, uint key_len,
 			enum ha_rkey_function find_flag)
 {
   DBUG_ASSERT(inited==INDEX);
-  graph->row_ref((void*) ref);	// reset before we have a cursor, so the memory is inited, avoiding the sefgault in position() when select with order by (bug #1133093)
+  // reset before we have a cursor, so the memory is not junk, avoiding the sefgault in position() when select with order by (bug #1133093)
+  graph->init_row_ref(ref);
   return index_read_idx(buf, active_index, key, key_len, find_flag);
 }
 
@@ -1107,6 +1108,15 @@ int ha_oqgraph::delete_all_rows()
 
 int ha_oqgraph::external_lock(THD *thd, int lock_type)
 {
+  // This method is also called to _unlock_ (lock_type == F_UNLCK)
+  // Which means we need to release things before we let the underlying backing table lock go...
+  if (lock_type == F_UNLCK) {
+    // If we have an index open on the backing table, we need to close it out here
+    // this means destroying any open cursor first.
+    // Then we can let the unlock go through to the backing table
+    graph->release_cursor();
+  }
+
   return edges->file->ha_external_lock(thd, lock_type);
 }