CBCC-1280: fix memory leak while tracing query

Change-Id: Id79b35592740e162a2d18ba09bdf3d24d477970c Reviewed-on: http://review.couchbase.org/c/libcouchbase/+/157429 Tested-by: Build Bot <build@couchbase.com> Reviewed-by: Sergey Avseyev <sergey.avseyev@gmail.com>
couchbase · Jul 14, 2021 · 0b93b1b · 0b93b1b
1 parent 2523348
commit 0b93b1b
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 24 deletions.
diff --git a/src/n1ql/query_handle.cc b/src/n1ql/query_handle.cc
@@ -70,14 +70,13 @@ static void prepare_rowcb(lcb_INSTANCE *instance, int, const lcb_RESPQUERY *row)
     origreq->cancel_prepare_query();
 
     if (row->ctx.rc != LCB_SUCCESS || (row->rflags & LCB_RESP_F_FINAL)) {
-        origreq->fail_prepared(row, row->ctx.rc);
+        return origreq->fail_prepared(row, row->ctx.rc);
     } else {
         // Insert into cache
         Json::Value prepared;
         if (!Json::Reader().parse(row->row, row->row + row->nrow, prepared)) {
             lcb_log(LOGARGS2(instance, ERROR), LOGFMT "Invalid JSON returned from PREPARE", LOGID(origreq));
-            origreq->fail_prepared(row, LCB_ERR_PROTOCOL_ERROR);
-            return;
+            return origreq->fail_prepared(row, LCB_ERR_PROTOCOL_ERROR);
         }
 
         bool eps = LCBVB_CCAPS(LCBT_VBCONFIG(instance)) & LCBVB_CCAP_N1QL_ENHANCED_PREPARED_STATEMENTS;
@@ -89,7 +88,7 @@ static void prepare_rowcb(lcb_INSTANCE *instance, int, const lcb_RESPQUERY *row)
         // Issue the query with the newly prepared plan
         lcb_STATUS rc = origreq->apply_plan(ent);
         if (rc != LCB_SUCCESS) {
-            origreq->fail_prepared(row, rc);
+            return origreq->fail_prepared(row, rc);
         }
     }
 }
@@ -215,7 +214,6 @@ void lcb_QUERY_HANDLE_::invoke_row(lcb_RESPQUERY *resp, bool is_last)
         }
         resp->ctx.first_error_code = first_error_code;
         LCBTRACE_ADD_RETRIES(span_, retries_);
-        LCBTRACE_HTTP_FINISH(span_);
     }
     if (callback_) {
         callback_(instance_, LCB_CALLBACK_QUERY, resp);
@@ -403,8 +401,6 @@ lcb_STATUS lcb_QUERY_HANDLE_::issue_htreq(const std::string &body)
         lcb_cmdhttp_password(htcmd, password_.c_str(), password_.size());
     }
 
-    LCBTRACE_HTTP_START(instance_->settings, client_context_id.c_str(), span_, LCBTRACE_TAG_SERVICE_N1QL,
-                        LCBTRACE_THRESHOLD_QUERY, span_);
     lcb_cmdhttp_parent_span(htcmd, span_);
 
     rc = lcb_http(instance_, this, htcmd);
@@ -538,7 +534,9 @@ lcb_QUERY_HANDLE_::lcb_QUERY_HANDLE_(lcb_INSTANCE *obj, void *user_cookie, const
         }
     }
     if (instance_->settings->tracer) {
-        span_ = cmd->parent_span();
+        parent_span_ = cmd->parent_span();
+        LCBTRACE_HTTP_START(instance_->settings, client_context_id.c_str(), parent_span_, LCBTRACE_TAG_SERVICE_N1QL,
+                            LCBTRACE_THRESHOLD_QUERY, span_);
     }
 }
 
@@ -548,6 +546,7 @@ lcb_QUERY_HANDLE_::~lcb_QUERY_HANDLE_()
         lcb_RESPQUERY resp{};
         invoke_row(&resp, true);
     }
+    LCBTRACE_HTTP_FINISH(span_);
 
     if (http_request_) {
         record_http_op_latency(nullptr, "query", instance_, http_request_->start);

diff --git a/src/n1ql/query_handle.hh b/src/n1ql/query_handle.hh
@@ -285,6 +285,7 @@ struct lcb_QUERY_HANDLE_ : lcb::jsparse::Parser::Actions {
     std::int64_t last_config_revision_{0};
     bool idempotent_{false};
 
+    lcbtrace_SPAN *parent_span_{nullptr};
     lcbtrace_SPAN *span_{nullptr};
 
     lcb::io::Timer<lcb_QUERY_HANDLE_, &lcb_QUERY_HANDLE_::on_timeout> timeout_timer_;

diff --git a/src/tracing/span.cc b/src/tracing/span.cc
@@ -406,9 +406,7 @@ using namespace lcb::trace;
 
 Span::Span(lcbtrace_TRACER *tracer, const char *opname, uint64_t start, lcbtrace_REF_TYPE ref, lcbtrace_SPAN *other,
            void *external_span)
-    : m_tracer(tracer), m_opname(opname), m_finish(0), m_extspan(external_span), m_is_outer(false),
-      m_is_dispatch(false), m_is_encode(false), m_should_finish(true), m_svc(LCBTRACE_THRESHOLD__MAX),
-      m_svc_string(nullptr), m_total_dispatch(0), m_last_dispatch(0), m_total_server(0), m_last_server(0), m_encode(0)
+    : m_tracer(tracer), m_opname(opname), m_extspan(external_span)
 {
     if (other != nullptr && ref == LCBTRACE_REF_CHILD_OF) {
         m_parent = other;

diff --git a/src/tracing/tracing-internal.h b/src/tracing/tracing-internal.h
@@ -37,7 +37,6 @@ class Span
   public:
     Span(lcbtrace_TRACER *tracer, const char *opname, uint64_t start, lcbtrace_REF_TYPE ref, lcbtrace_SPAN *other,
          void *external_span);
-    // Span(lcbtrace_TRACER *tracer, const char *opname, lcbtrace_SPAN *other);
     ~Span();
 
     void finish(uint64_t finish);
@@ -77,22 +76,22 @@ class Span
     std::string m_opname;
     uint64_t m_span_id;
     uint64_t m_start;
-    uint64_t m_finish;
+    uint64_t m_finish{0};
     bool m_orphaned;
     Span *m_parent;
     void *m_extspan;
     sllist_root m_tags{};
-    bool m_is_outer;
-    bool m_is_dispatch;
-    bool m_is_encode;
-    bool m_should_finish;
-    lcbtrace_THRESHOLDOPTS m_svc;
-    const char *m_svc_string;
-    uint64_t m_total_dispatch;
-    uint64_t m_last_dispatch;
-    uint64_t m_total_server;
-    uint64_t m_last_server;
-    uint64_t m_encode;
+    bool m_is_outer{false};
+    bool m_is_dispatch{false};
+    bool m_is_encode{false};
+    bool m_should_finish{true};
+    lcbtrace_THRESHOLDOPTS m_svc{LCBTRACE_THRESHOLD__MAX};
+    const char *m_svc_string{nullptr};
+    uint64_t m_total_dispatch{0};
+    uint64_t m_last_dispatch{0};
+    uint64_t m_total_server{0};
+    uint64_t m_last_server{0};
+    uint64_t m_encode{0};
 };
 
 struct ReportedSpan {