From 4973695c2ac1ac0ccf624cdbb2e45aabc3593565 Mon Sep 17 00:00:00 2001
From: =?utf8?q?=EB=B0=95=EC=B2=9C=EA=B5=90/On-Device=20Lab=28SR=29/Enginee?=
 =?utf8?q?r/=EC=82=BC=EC=84=B1=EC=A0=84=EC=9E=90?= <ch.bahk@samsung.com>
Date: Mon, 13 May 2019 16:46:07 +0900
Subject: [PATCH] [locomotiv] Session frees node annotations on destruction
 (#3441)

* [locomotiv] Session frees node annotations on destruction

This commit introduces destructor of Session. Previously node
annotations set by Session, like NodeDataImpl, remain even after
Session closed. They were not memory leak by definition, as they can
be accessed through another Session. However node annotations maybe
preserved until program ends, so it is like memory leak. This commit
would prevent that situation.

Signed-off-by: Cheongyo Bahk <ch.bahk@samsung.com>

* Delete unused using Index

* Session is final class
---
 contrib/locomotiv/include/locomotiv/Session.h   |  5 +-
 contrib/locomotiv/src/Session.cpp               |  9 ++++
 contrib/locomotiv/src/Session.internal.test.cpp | 62 +++++++++++++++++++++++++
 3 files changed, 75 insertions(+), 1 deletion(-)
 create mode 100644 contrib/locomotiv/src/Session.internal.test.cpp

diff --git a/contrib/locomotiv/include/locomotiv/Session.h b/contrib/locomotiv/include/locomotiv/Session.h
index 6bca285..4fb913d 100644
--- a/contrib/locomotiv/include/locomotiv/Session.h
+++ b/contrib/locomotiv/include/locomotiv/Session.h
@@ -29,7 +29,7 @@ namespace locomotiv
 /**
  * @brief Session for loco graph inference
  */
-class Session
+class Session final
 {
 public:
   Session() = delete;
@@ -38,6 +38,9 @@ public:
     // DO NOTHING
   }
 
+  /// @brief Free all node annotations of the graph assigned by this Session
+  ~Session();
+
   /**
    * @brief Set graph input at specific index by NodeData.
    *
diff --git a/contrib/locomotiv/src/Session.cpp b/contrib/locomotiv/src/Session.cpp
index 22ba44c..8bccbda 100644
--- a/contrib/locomotiv/src/Session.cpp
+++ b/contrib/locomotiv/src/Session.cpp
@@ -23,6 +23,15 @@
 namespace locomotiv
 {
 
+Session::~Session()
+{
+  for (uint32_t i = 0; i < _graph->nodes()->size(); ++i)
+  {
+    auto node = _graph->nodes()->at(i);
+    erase_annot_data(node);
+  }
+}
+
 bool Session::set_input(uint32_t index, std::unique_ptr<NodeData> &&data)
 {
   // Check whether already annotated
diff --git a/contrib/locomotiv/src/Session.internal.test.cpp b/contrib/locomotiv/src/Session.internal.test.cpp
new file mode 100644
index 0000000..ed7d08d
--- /dev/null
+++ b/contrib/locomotiv/src/Session.internal.test.cpp
@@ -0,0 +1,62 @@
+/*
+ * Copyright (c) 2019 Samsung Electronics Co., Ltd. All Rights Reserved
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "locomotiv/Session.h"
+#include "locomotiv/NodeData.h"
+
+// This flie is internal test because it includes this local header
+#include "NodeDataImpl.h"
+
+#include <loco.h>
+#include <nncc/core/ADT/tensor/Shape.h>
+#include <nncc/core/ADT/tensor/Buffer.h>
+#include <nncc/core/ADT/tensor/LexicalLayout.h>
+
+#include <gtest/gtest.h>
+
+using nncc::core::ADT::tensor::Shape;
+using nncc::core::ADT::tensor::LexicalLayout;
+using nncc::core::ADT::tensor::make_buffer;
+
+TEST(Session, dtor)
+{
+  auto g = loco::make_graph();
+
+  // Pull node
+  auto pull = g->nodes()->create<loco::Pull>();
+  pull->dtype(loco::DataType::FLOAT32);
+  pull->rank(1);
+  pull->dim(0) = loco::make_dimension(1);
+
+  // Input
+  auto input = g->inputs()->create();
+  input->node(pull);
+
+  {
+    locomotiv::Session s(g.get());
+
+    auto buf = make_buffer<float, LexicalLayout>(Shape{1});
+    auto data = locomotiv::make_data(buf);
+
+    s.set_input(0, std::move(data));
+
+    auto data_annotated = locomotiv::annot_data(input->node());
+    ASSERT_NE(data_annotated, nullptr);
+  }
+
+  auto data_annotated = locomotiv::annot_data(input->node());
+  ASSERT_EQ(data_annotated, nullptr);
+}
-- 
2.7.4