Merge pull request #15 from usdot-jpo-ode/memleak

Memleak

include/bsmfilter.hpp

@@ -521,7 +521,11 @@ class BSMHandler {
         const double get_box_extension() const;
 
     private:
-        rapidjson::Document document_;              ///< JSON DOM
+
+        // JMC: The leak seems to be caused by re-using the RapidJSON document instance.
+        // JMC: We will use a unique instance for each message.
+        // rapidjson::Document document_;              ///< JSON DOM
+
         uint32_t activated_;                        ///< A flag word indicating which features of the privacy protection are activiated.
 
         bool finalized_;                            ///< Indicates the JSON string after redaction has been created and retrieved.

src/bsmfilter.cpp

@@ -285,7 +285,6 @@ BSMHandler::ResultStringMap BSMHandler::result_string_map{
         };
 
 BSMHandler::BSMHandler(Quad::Ptr quad_ptr, const ConfigMap& conf ):
-    document_{},
     activated_{0},
     result_{ ResultStatus::SUCCESS },
     bsm_{},
@@ -366,31 +365,34 @@ bool BSMHandler::process( const std::string& bsm_json ) {
     double latitude = 0.0;
     double longitude = 0.0;
     std::string id;
+
+    // JMC: Attempt to fix memory leak; build and destroy JSON object each time to ensure memory is reclaimed.
+    rapidjson::Document document;
 
     finalized_ = false;
     result_ = ResultStatus::SUCCESS;
 
     // create the DOM
     // check for errors
-    if (document_.Parse(bsm_json.c_str()).HasParseError()) {
+    if (document.Parse(bsm_json.c_str()).HasParseError()) {
         result_ = ResultStatus::PARSE;
 
         return false;
     }
 
-    if (!document_.IsObject()) {
+    if (!document.IsObject()) {
         result_ = ResultStatus::PARSE;
 
         return false;
     }
 
-    if (!document_.HasMember("metadata")) {
+    if (!document.HasMember("metadata")) {
         result_ = ResultStatus::MISSING;
 
         return false;
     }
 
-    rapidjson::Value& metadata = document_["metadata"];
+    rapidjson::Value& metadata = document["metadata"];
 
     // switch sanitized flag
     if (!metadata.HasMember("sanitized")) {
@@ -423,13 +425,13 @@ bool BSMHandler::process( const std::string& bsm_json ) {
     std::string payload_type_str = metadata["payloadType"].GetString();
 
     if (payload_type_str == "us.dot.its.jpo.ode.model.OdeBsmPayload") {
-        if (!document_.HasMember("payload")) {
+        if (!document.HasMember("payload")) {
             result_ = ResultStatus::MISSING;
 
             return false;
         }
 
-        rapidjson::Value& payload = document_["payload"];
+        rapidjson::Value& payload = document["payload"];
 
         // handle BSM payload
         if (!payload.HasMember("data")) {
@@ -515,7 +517,7 @@ bool BSMHandler::process( const std::string& bsm_json ) {
             bsm_.set_original_id(id);
             idr_(id);
 
-            core_data["id"].SetString(id.c_str(), static_cast<rapidjson::SizeType>(id.size()), document_.GetAllocator());
+            core_data["id"].SetString(id.c_str(), static_cast<rapidjson::SizeType>(id.size()), document.GetAllocator());
         }
 
         bsm_.set_id(id);
@@ -587,6 +589,17 @@ bool BSMHandler::process( const std::string& bsm_json ) {
         return false;
     }
 
+    // JMC: Moving this here to finalize the json string instead of in get_json()
+    // JMC: Go ahead and write out the BSM in redacted form using the document that we built in
+    // JMC: this method.
+    rapidjson::StringBuffer buffer;
+    rapidjson::Writer<rapidjson::StringBuffer> writer(buffer);
+    document.Accept(writer);
+    json_ = buffer.GetString();
+
+    // TODO: if we keep this model, this variable serves no purpose.
+    finalized_ = true;
+
     return result_ == ResultStatus::SUCCESS;
 }
 
@@ -603,22 +616,30 @@ BSM& BSMHandler::get_bsm() {
 }
 
 const std::string& BSMHandler::get_json() {
-    rapidjson::StringBuffer buffer;
-    rapidjson::Writer<rapidjson::StringBuffer> writer(buffer);
-    document_.Accept(writer);
+    // rapidjson::StringBuffer buffer;
+    // rapidjson::Writer<rapidjson::StringBuffer> writer(buffer);
+    // TODO: The leak is caused by REUSING this document_ instance.  Something in it isn't getting freed.
+    // document_.Accept(writer);
 
-    json_ = buffer.GetString();
+    // json_ = buffer.GetString();
 
-    finalized_ = true;
+    // finalized_ = true;
 
+    // JMC: The json_ string is set in the process method now to avoid the memory leak in RapidJSON.
+    // IMPORTANT: Ensure you call the process method prior to attempting to call this method.
+    // IMPORTANT: This is what happens in the main loop of the ppm code.
     return json_;
 }
 
 std::string::size_type BSMHandler::get_bsm_buffer_size() {
-    if ( !finalized_ ) {
-        get_json();
-    }
-
+    // JMC: how we are using this it is always finalized now that I moved the document object.
+    // if ( !finalized_ ) {
+    //     get_json();
+    // }
+
+    // JMC: The json_ string is set in the process method now to avoid the memory leak in RapidJSON.
+    // IMPORTANT: Ensure you call the process method prior to attempting to call this method.
+    // IMPORTANT: This is what happens in the main loop of the ppm code.
     return json_.size();
 }
 

src/ppm.cpp

@@ -768,6 +768,7 @@ int PPM::operator()(void) {
             continue;
         }
 
+        // JMC: There was leak in here caused by RapidJSON.  It has been fixed.  The notes are in that class's code.
         BSMHandler handler{qptr, pconf};
 
         std::vector<RdKafka::TopicPartition*> partitions;