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

3 files changed, 45 insertions, 3 deletions

diff --git a/storage/oqgraph/graphcore.cc b/storage/oqgraph/graphcore.cc
index 30b7e907f9e..ce785b205c3 100644
--- a/storage/oqgraph/graphcore.cc
+++ b/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()
diff --git a/storage/oqgraph/graphcore.h b/storage/oqgraph/graphcore.h
index b8a90407da8..f7eff77d8b1 100644
--- a/storage/oqgraph/graphcore.h
+++ b/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;
diff --git a/storage/oqgraph/ha_oqgraph.cc b/storage/oqgraph/ha_oqgraph.cc
index 537d5b49205..49e0df0fc86 100644
--- a/storage/oqgraph/ha_oqgraph.cc
+++ b/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);
 }