MYCPP-271: Add missing virtual dtors and other memory leak fixes.

Author: rsomla1

devapi/collection_crud.cc

@@ -133,9 +133,9 @@ class Op_collection_add
   }
 
 
-  internal::BaseResult get_result() override
+  internal::BaseResult mk_result(cdk::Reply *reply) override
   {
-    return Result::Access::mk(m_sess, m_reply, m_id_list);
+    return Result::Access::mk(m_sess, reply, m_id_list);
   }
 
 
@@ -339,15 +339,14 @@ class Op_collection_remove
 
   cdk::Reply* send_command()
   {
-    m_reply =
-        new cdk::Reply(get_cdk_session().coll_remove(
-                              m_coll,
-                              get_where(),
-                              get_order_by(),
-                              get_limit(),
-                              get_params()
-                      ));
-    return m_reply;
+    return
+      new cdk::Reply(get_cdk_session().coll_remove(
+                            m_coll,
+                            get_where(),
+                            get_order_by(),
+                            get_limit(),
+                            get_params()
+                    ));
   }
 
   friend mysqlx::CollectionRemove;
@@ -405,18 +404,17 @@ class Op_collection_find
 
   cdk::Reply* send_command()
   {
-    m_reply =
-        new cdk::Reply(get_cdk_session().coll_find(
-                            m_coll,
-                            get_where(),
-                            get_doc_proj(),
-                            get_order_by(),
-                            nullptr,  // group_by
-                            nullptr,  // having
-                            get_limit(),
-                            get_params()
-                      ));
-    return m_reply;
+    return
+      new cdk::Reply(get_cdk_session().coll_find(
+                          m_coll,
+                          get_where(),
+                          get_doc_proj(),
+                          get_order_by(),
+                          nullptr,  // group_by
+                          nullptr,  // having
+                          get_limit(),
+                          get_params()
+                    ));
   }
 
   friend mysqlx::CollectionFind;
@@ -509,7 +507,7 @@ class Op_collection_modify
     if (m_update.empty())
       return NULL;
 
-    m_reply =
+    return
       new cdk::Reply(get_cdk_session().coll_update(
                        m_coll,
                        get_where(),
@@ -518,7 +516,6 @@ class Op_collection_modify
                        get_limit(),
                        get_params()
                      ));
-    return m_reply;
   }
 
   void add_operation(Field_Op::Operation op,

devapi/impl.h

@@ -518,7 +518,7 @@ struct internal::BaseResult::Access
      underlying CDK session. This produces a cdk::Reply object which
      is used for further processing. Sending the CRUD operation is
      performed by method `send_command` which should be overwriten by
-     derived class. Derived class has access to underlying CDK session
+     derived class. Derived class has access to the underlying CDK session
      with method `get_cdk_session()`.
 
   2. After getting cdk::Reply object implementation waits for it to
@@ -541,7 +541,12 @@ class Op_base
 protected:
 
   internal::XSession_base   *m_sess;
-  cdk::Reply *m_reply = NULL;
+
+  /*
+    Note: using cdk::scoped_ptr to be able to trnasfer ownership
+    to a different object.
+  */
+  cdk::scoped_ptr<cdk::Reply> m_reply;
 
   row_count_t m_limit = 0;
   bool m_has_limit = false;
@@ -562,6 +567,8 @@ class Op_base
     : m_sess(&tbl.getSession())
   {}
 
+  virtual ~Op_base()
+  {}
 
   cdk::Session& get_cdk_session()
   {
@@ -571,6 +578,21 @@ class Op_base
 
   virtual cdk::Reply* send_command() = 0;
 
+  /*
+    Given cdk reply object for the statement, return BaseResult object
+    that handles that reply. The reply pointer can be NULL in case no
+    reply has been generated for the statement (TODO: explain in what
+    scenario reply can be NULL).
+
+    The returned BaseResult object should take ownership of the cdk reply
+    object passed here (if any).
+  */
+
+  virtual internal::BaseResult mk_result(cdk::Reply *reply)
+  {
+    return reply ? internal::BaseResult::Access::mk(m_sess, reply)
+      : internal::BaseResult::Access::mk_empty();
+  }
 
   // Limit and offset
 
@@ -620,7 +642,7 @@ class Op_base
     if (m_inited)
       return;
     m_inited = true;
-    m_reply = send_command();
+    m_reply.reset(send_command());
   }
 
   bool is_completed()
@@ -629,7 +651,7 @@ class Op_base
       return true;
 
     init();
-    m_completed = (NULL == m_reply) || m_reply->is_completed();
+    m_completed = (!m_reply) || m_reply->is_completed();
     return m_completed;
   }
 
@@ -654,17 +676,17 @@ class Op_base
     return get_result();
   }
 
-  virtual internal::BaseResult get_result()
+  internal::BaseResult get_result()
   {
     if (!is_completed())
       THROW("Attempt to get result of incomplete operation");
 
-    // Note: BaseResult takes ownership of the cdk::Reply object.
+    /*
+      Note: result created by mk_result() takes ownership of the cdk::Reply
+      object.
+    */
 
-    cdk::Reply *reply = m_reply;
-    m_reply = NULL;
-    return reply ? internal::BaseResult::Access::mk(m_sess, reply)
-                 : internal::BaseResult::Access::mk_empty();
+    return mk_result(m_reply.release());
   }
 
 

devapi/result.cc

@@ -1273,6 +1273,10 @@ internal::BaseResult::~BaseResult()
   try {
     if (m_sess)
       m_sess->deregister_result(this);
+  }
+  catch (...) {}
+
+  try {
     if (m_owns_impl)
       delete m_impl;
   }
@@ -1282,6 +1286,9 @@ internal::BaseResult::~BaseResult()
 
 void mysqlx::internal::BaseResult::init(mysqlx::internal::BaseResult &&init_)
 {
+  if (m_impl && m_owns_impl)
+    delete m_impl;
+
   m_pos = 0;
   m_impl = init_.m_impl;
   if (!init_.m_owns_impl)

include/devapi/statement.h

@@ -55,6 +55,7 @@ class XSession_base;
 struct Executable_impl
 {
   virtual BaseResult execute() = 0;
+  virtual ~Executable_impl() {}
 };
 
 }  // internal