CCBC-972: Fix memory issues reported by valgrind

Change-Id: I0d4b9ffaa8dd8927168e1675cd0175aaa34d08bd Reviewed-on: http://review.couchbase.org/99424 Tested-by: Build Bot <build@couchbase.com> Tested-by: Ellis Breen <ellis.breen@couchbase.com> Reviewed-by: Ellis Breen <ellis.breen@couchbase.com>
couchbase · Sep 11, 2018 · de5c900 · de5c900
1 parent 638c578
commit de5c900
Show file tree

Hide file tree

Showing 10 changed files with 47 additions and 17 deletions.
diff --git a/src/handler.cc b/src/handler.cc
@@ -679,7 +679,6 @@ H_observe(mc_PIPELINE *pipeline, mc_PACKET *request, MemcachedResponse *response
             rd->procs->handler(pipeline, request, resp.rc, &resp);
         }
     }
-    TRACE_OBSERVE_END(root, request, response);
 }
 
 static void

diff --git a/src/instance.cc b/src/instance.cc
@@ -618,6 +618,15 @@ void lcb_destroy(lcb_t instance)
     DESTROY(lcb_vbguess_destroy, vbguess);
     DESTROY(lcb_n1qlcache_destroy, n1ql_cache);
 
+    if (instance->cmdq.pipelines) {
+        unsigned ii;
+        for (ii = 0; ii < instance->cmdq.npipelines; ii++) {
+            lcb::Server *server = static_cast<lcb::Server*>(instance->cmdq.pipelines[ii]);
+            if (server) {
+                server->instance = NULL;
+            }
+        }
+    }
     mcreq_queue_cleanup(&instance->cmdq);
     lcb_aspend_cleanup(po);
 
@@ -686,7 +695,7 @@ lcb_st::find_server(const lcb_host_t& host) const
     unsigned ii;
     for (ii = 0; ii < cmdq.npipelines; ii++) {
         lcb::Server *server = static_cast<lcb::Server*>(cmdq.pipelines[ii]);
-        if (lcb_host_equals(&server->get_host(), &host)) {
+        if (server && lcb_host_equals(&server->get_host(), &host)) {
             return server;
         }
     }

diff --git a/src/mc/compress.cc b/src/mc/compress.cc
@@ -54,18 +54,25 @@ class FragBufSource : public snappy::Source
 
     virtual void Skip(size_t n)
     {
-        lcb_IOV &v = buf->iov[idx];
-        if (ptr == static_cast< const char * >(v.iov_base) && n == v.iov_len) {
-            ptr = static_cast< const char * >(buf->iov[++idx].iov_base);
-        } else {
-            ptr += n;
-            if (static_cast< size_t >(ptr - static_cast< const char * >(v.iov_base)) == v.iov_len) {
-                ptr = static_cast< const char * >(buf->iov[++idx].iov_base);
+        do {
+            size_t spanleft = buf->iov[idx].iov_len - (ptr - static_cast< const char * >(buf->iov[idx].iov_base));
+            if (n < spanleft) {
+                ptr += n;
+                left -= n;
+                break;
             }
-        }
-        left -= n;
-        if (left == 0) {
+            if (idx + 1 >= buf->niov) {
+                left = 0;
+                ptr = NULL;
+                break;
+            }
+            left -= spanleft;
+            n -= spanleft;
+            ptr = static_cast< const char * >(buf->iov[++idx].iov_base);
+        } while (n > 0);
+        if (left == 0 || idx >= buf->niov) {
             ptr = NULL;
+            left = 0;
         }
     }
 

diff --git a/src/mc/mcreq.c b/src/mc/mcreq.c
@@ -640,6 +640,8 @@ mcreq_queue_cleanup(mc_CMDQUEUE *queue)
     }
     free(queue->scheds);
     free(queue->pipelines);
+    queue->pipelines = NULL;
+    queue->npipelines = 0;
     queue->scheds = NULL;
 }
 

diff --git a/src/mcserver/mcserver.cc b/src/mcserver/mcserver.cc
@@ -876,6 +876,18 @@ Server::~Server() {
         return;
     }
 
+    if (this->instance) {
+        unsigned ii;
+        mc_CMDQUEUE *cmdq = &this->instance->cmdq;
+        for (ii = 0; ii < cmdq->npipelines; ii++) {
+            lcb::Server *server = static_cast<lcb::Server*>(cmdq->pipelines[ii]);
+            if (server == this) {
+                cmdq->pipelines[ii] = NULL;
+                break;
+            }
+        }
+    }
+    this->instance = NULL;
     mcreq_pipeline_cleanup(this);
 
     if (io_timer) {

diff --git a/src/mcserver/negotiate.cc b/src/mcserver/negotiate.cc
@@ -290,7 +290,7 @@ SessionRequestImpl::set_chosen_mech(std::string& mechlist,
             return MECH_UNAVAILABLE;
         }
         mechlist.assign(forcemech);
-        if (memcmp(forcemech, "SCRAM-SHA", sizeof("SCRAM-SHA") - 1) == 0) {
+        if (strncmp(forcemech, "SCRAM-SHA", sizeof("SCRAM-SHA") - 1) == 0) {
             allow_scram_sha = 1;
         }
     }

diff --git a/src/operations/observe.cc b/src/operations/observe.cc
@@ -133,6 +133,7 @@ static void handle_observe_callback(mc_PIPELINE *pl, mc_PACKET *pkt, lcb_error_t
     }
     opc->remaining--;
     if (opc->remaining == 0) {
+        TRACE_OBSERVE_END(instance, pkt);
         delete opc;
     }
 }

diff --git a/src/trace.h b/src/trace.h
@@ -96,8 +96,8 @@
 #define TRACE_OBSERVE_PROGRESS(instance, pkt, mcresp, resp)                                                            \
     TRACE(TRACE_END_COMMON(LIBCOUCHBASE_OBSERVE_PROGRESS, instance, pkt, mcresp, resp, (resp)->cas, (resp)->status,    \
                            (resp)->ismaster, (resp)->ttp, (resp)->ttr))
-#define TRACE_OBSERVE_END(instance, pkt, mcresp)                                                                       \
-    TRACE(LIBCOUCHBASE_OBSERVE_END(instance, mcresp->opaque(), mcresp->opcode(),                                       \
+#define TRACE_OBSERVE_END(instance, pkt)                                                                               \
+    TRACE(LIBCOUCHBASE_OBSERVE_END(instance, pkt->opaque, PROTOCOL_BINARY_CMD_OBSERVE,                                 \
                                    MCREQ_PKT_RDATA(pkt)->dispatch - MCREQ_PKT_RDATA(pkt)->start, LCB_SUCCESS))
 
 #define TRACE_HTTP_BEGIN(req)                                                                                          \

diff --git a/src/tracing/threshold_logging_tracer.cc b/src/tracing/threshold_logging_tracer.cc
@@ -58,7 +58,7 @@ static void tlt_report(lcbtrace_TRACER *wrapper, lcbtrace_SPAN *span)
     char *value = NULL;
     size_t nvalue;
     if (lcbtrace_span_get_tag_str(span, LCBTRACE_TAG_SERVICE, &value, &nvalue) == LCB_SUCCESS) {
-        if (memcmp(value, LCBTRACE_TAG_SERVICE_KV, nvalue) == 0) {
+        if (strncmp(value, LCBTRACE_TAG_SERVICE_KV, nvalue) == 0) {
             if (lcbtrace_span_is_orphaned(span)) {
                 tracer->add_orphan(span);
             } else {

diff --git a/tests/iotests/mock-environment.cc b/tests/iotests/mock-environment.cc
@@ -415,7 +415,7 @@ void MockEnvironment::bootstrapRealCluster()
 
 extern "C" {
 static void mock_flush_callback(lcb_t, int, const lcb_RESPBASE *resp) {
-    assert(resp->rc == LCB_SUCCESS);
+    ASSERT_EQ(LCB_SUCCESS, resp->rc);
 }
 }