You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mxnet.apache.org by GitBox <gi...@apache.org> on 2017/11/30 19:52:26 UTC

[GitHub] piiswrong closed pull request #8737: Use RAII and fix Coverity resource leaks #10371 and others

piiswrong closed pull request #8737: Use RAII and fix Coverity resource leaks #10371 and others
URL: https://github.com/apache/incubator-mxnet/pull/8737
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/cpp-package/example/alexnet.cpp b/cpp-package/example/alexnet.cpp
index dd5d2b4b06..f66ef02a93 100644
--- a/cpp-package/example/alexnet.cpp
+++ b/cpp-package/example/alexnet.cpp
@@ -215,7 +215,7 @@ int main(int argc, char const *argv[]) {
   args_map["label"] = NDArray(Shape(batch_size), ctx);
 
   /*with data and label, executor can be generated automatically*/
-  auto *exec = Net.SimpleBind(ctx, args_map);
+  auto exec = Net.SimpleBind(ctx, args_map);
   auto arg_names = Net.ListArguments();
   aux_map = exec->aux_dict();
   args_map = exec->arg_dict();
@@ -321,7 +321,6 @@ int main(int argc, char const *argv[]) {
     NDArray::Save(save_path_param, save_args);
   }
   /*don't foget to release the executor*/
-  delete exec;
   MXNotifyShutdown();
   return 0;
 }
diff --git a/cpp-package/example/charRNN.cpp b/cpp-package/example/charRNN.cpp
index 218d11efc9..2dc4aa892a 100644
--- a/cpp-package/example/charRNN.cpp
+++ b/cpp-package/example/charRNN.cpp
@@ -453,14 +453,14 @@ void train(const string file, int batch_size, int max_epoch, int start_epoch) {
   }
   vector<mx_float> zeros(batch_size * num_hidden, 0);
   // RNN.SimpleBind(device, args_map, {}, {{"data", kNullOp}});
-  Executor* exe = RNN.SimpleBind(device, args_map);
+  auto exe = RNN.SimpleBind(device, args_map);
 
   if (start_epoch == -1) {
     Xavier xavier = Xavier(Xavier::gaussian, Xavier::in, 2.34);
     for (auto &arg : exe->arg_dict())
       xavier(arg.first, &arg.second);
   } else {
-    LoadCheckpoint(prefix + "-" + to_string(start_epoch) + ".params", exe);
+    LoadCheckpoint(prefix + "-" + to_string(start_epoch) + ".params", exe.get());
   }
   start_epoch++;
 
@@ -499,7 +499,7 @@ void train(const string file, int batch_size, int max_epoch, int start_epoch) {
         chrono::duration_cast<chrono::seconds>(toc - tic).count() << " seconds ";
     OutputPerplexity(&exe->arg_dict()["softmax_label"], &exe->outputs[0]);
     string filepath = prefix + "-" + to_string(epoch) + ".params";
-    SaveCheckpoint(filepath, RNN, exe);
+    SaveCheckpoint(filepath, RNN, exe.get());
   }
 }
 
@@ -538,14 +538,14 @@ void trainWithBuiltInRNNOp(const string file, int batch_size, int max_epoch, int
   args_map["LSTM_init_h"] = NDArray(Shape(num_lstm_layer, batch_size, num_hidden), device, false);
   args_map["softmax_label"] = NDArray(Shape(batch_size, sequence_length_max), device, false);
   vector<mx_float> zeros(batch_size * num_lstm_layer * num_hidden, 0);
-  Executor* exe = RNN.SimpleBind(device, args_map);
+  auto exe = RNN.SimpleBind(device, args_map);
 
   if (start_epoch == -1) {
     RNNXavier xavier = RNNXavier(Xavier::gaussian, Xavier::in, 2.34);
     for (auto &arg : exe->arg_dict())
       xavier(arg.first, &arg.second);
   } else {
-    LoadCheckpoint(prefix + "-" + to_string(start_epoch) + ".params", exe);
+    LoadCheckpoint(prefix + "-" + to_string(start_epoch) + ".params", exe.get());
   }
   start_epoch++;
 
@@ -578,7 +578,7 @@ void trainWithBuiltInRNNOp(const string file, int batch_size, int max_epoch, int
         chrono::duration_cast<chrono::seconds>(toc - tic).count() << " seconds ";
     OutputPerplexity(&exe->arg_dict()["softmax_label"], &exe->outputs[0]);
     string filepath = prefix + "-" + to_string(epoch) + ".params";
-    SaveCheckpoint(filepath, RNN, exe);
+    SaveCheckpoint(filepath, RNN, exe.get());
   }
 }
 
@@ -602,8 +602,8 @@ void predict(wstring* ptext, int sequence_length, const string param_file,
     args_map[key + "c"].SyncCopyFromCPU(zeros);
     args_map[key + "h"].SyncCopyFromCPU(zeros);
   }
-  Executor* exe = RNN.SimpleBind(device, args_map);
-  LoadCheckpoint(param_file, exe);
+  auto exe = RNN.SimpleBind(device, args_map);
+  LoadCheckpoint(param_file, exe.get());
 
   mx_float index;
   wchar_t next = 0;
@@ -662,8 +662,8 @@ void predictWithBuiltInRNNOp(wstring* ptext, int sequence_length, const string p
   args_map["LSTM_init_h"] = NDArray(Shape(num_lstm_layer, 1, num_hidden), device, false);
   args_map["LSTM_init_c"].SyncCopyFromCPU(zeros);
   args_map["LSTM_init_h"].SyncCopyFromCPU(zeros);
-  Executor* exe = RNN.SimpleBind(device, args_map);
-  LoadCheckpoint(param_file, exe);
+  auto exe = RNN.SimpleBind(device, args_map);
+  LoadCheckpoint(param_file, exe.get());
 
   mx_float index;
   wchar_t next = 0;
diff --git a/cpp-package/example/googlenet.cpp b/cpp-package/example/googlenet.cpp
index fe5dea6a1f..67adcd76d0 100644
--- a/cpp-package/example/googlenet.cpp
+++ b/cpp-package/example/googlenet.cpp
@@ -149,7 +149,7 @@ int main(int argc, char const *argv[]) {
      ->SetParam("wd", weight_decay);
 
 
-  auto *exec = googlenet.SimpleBind(Context::gpu(), args_map);
+  auto exec = googlenet.SimpleBind(Context::gpu(), args_map);
   auto arg_names = googlenet.ListArguments();
 
   for (int iter = 0; iter < max_epoch; ++iter) {
@@ -182,7 +182,6 @@ int main(int argc, char const *argv[]) {
     LG << "Accuracy: " << acu.Get();
   }
 
-  delete exec;
   MXNotifyShutdown();
   return 0;
 }
diff --git a/cpp-package/example/inception_bn.cpp b/cpp-package/example/inception_bn.cpp
index e6f47904e0..01dbba3d89 100644
--- a/cpp-package/example/inception_bn.cpp
+++ b/cpp-package/example/inception_bn.cpp
@@ -176,7 +176,7 @@ int main(int argc, char const *argv[]) {
      ->SetParam("lr", learning_rate)
      ->SetParam("wd", weight_decay);
 
-  auto *exec = inception_bn_net.SimpleBind(Context::gpu(), args_map);
+  auto exec = inception_bn_net.SimpleBind(Context::gpu(), args_map);
   auto arg_names = inception_bn_net.ListArguments();
 
   for (int iter = 0; iter < max_epoch; ++iter) {
@@ -212,7 +212,6 @@ int main(int argc, char const *argv[]) {
     }
     LG << "Accuracy: " << acu.Get();
   }
-  delete exec;
   MXNotifyShutdown();
   return 0;
 }
diff --git a/cpp-package/example/lenet.cpp b/cpp-package/example/lenet.cpp
index 4c5a1f1165..f0cba090cb 100644
--- a/cpp-package/example/lenet.cpp
+++ b/cpp-package/example/lenet.cpp
@@ -138,7 +138,7 @@ class Lenet {
        ->SetParam("lr", learning_rate)
        ->SetParam("wd", weight_decay);
 
-    Executor *exe = lenet.SimpleBind(ctx_dev, args_map);
+    auto exe = lenet.SimpleBind(ctx_dev, args_map);
     auto arg_names = lenet.ListArguments();
 
     for (int ITER = 0; ITER < max_epoch; ++ITER) {
@@ -168,7 +168,6 @@ class Lenet {
       LG << "Iter " << ITER
          << ", accuracy: " << ValAccuracy(batch_size * 10, lenet);
     }
-    delete exe;
   }
 
  private:
@@ -218,7 +217,7 @@ class Lenet {
       start_index += batch_size;
       NDArray::WaitAll();
 
-      Executor *exe = lenet.SimpleBind(ctx_dev, args_map);
+      auto exe = lenet.SimpleBind(ctx_dev, args_map);
       exe->Forward(false);
 
       const auto &out = exe->outputs;
@@ -244,8 +243,6 @@ class Lenet {
         if (label == p_label) correct_count++;
       }
       all_count += batch_size;
-
-      delete exe;
     }
     return correct_count * 1.0 / all_count;
   }
diff --git a/cpp-package/example/lenet_with_mxdataiter.cpp b/cpp-package/example/lenet_with_mxdataiter.cpp
index 04f5cbca3a..6e89541df2 100644
--- a/cpp-package/example/lenet_with_mxdataiter.cpp
+++ b/cpp-package/example/lenet_with_mxdataiter.cpp
@@ -106,7 +106,7 @@ int main(int argc, char const *argv[]) {
      ->SetParam("wd", weight_decay);
 
 
-  auto *exec = lenet.SimpleBind(Context::gpu(), args_map);
+  auto exec = lenet.SimpleBind(Context::gpu(), args_map);
   auto arg_names = lenet.ListArguments();
 
   // Create metrics
@@ -167,7 +167,6 @@ int main(int argc, char const *argv[]) {
     LG << "Epoch[" << iter << "] Val-Accuracy=" << val_acc.Get();
   }
 
-  delete exec;
   MXNotifyShutdown();
   return 0;
 }
diff --git a/cpp-package/example/mlp.cpp b/cpp-package/example/mlp.cpp
index b40328da6e..223cac9251 100644
--- a/cpp-package/example/mlp.cpp
+++ b/cpp-package/example/mlp.cpp
@@ -141,7 +141,7 @@ void MLP() {
   std::vector<NDArray> aux_states;
 
   cout << "make the Executor" << endl;
-  Executor* exe = new Executor(sym_out, ctx_dev, in_args, arg_grad_store,
+  auto exe = new Executor(sym_out, ctx_dev, in_args, arg_grad_store,
                                grad_req_type, aux_states);
 
   cout << "Training" << endl;
@@ -168,7 +168,6 @@ void MLP() {
     NDArray::WaitAll();
   }
 
-  delete exe;
   delete[] aptr_x;
   delete[] aptr_y;
 }
diff --git a/cpp-package/example/mlp_cpu.cpp b/cpp-package/example/mlp_cpu.cpp
index 051bad1bd2..fabe32040d 100644
--- a/cpp-package/example/mlp_cpu.cpp
+++ b/cpp-package/example/mlp_cpu.cpp
@@ -93,7 +93,7 @@ int main(int argc, char** argv) {
      ->SetParam("wd", weight_decay);
 
   // Create executor by binding parameters to the model
-  auto *exec = net.SimpleBind(ctx, args);
+  auto exec = net.SimpleBind(ctx, args);
   auto arg_names = net.ListArguments();
 
   // Start training
@@ -134,7 +134,6 @@ int main(int argc, char** argv) {
     LG << "Epoch: " << iter << " " << samples/duration << " samples/sec Accuracy: " << acc.Get();
   }
 
-  delete exec;
   MXNotifyShutdown();
   return 0;
 }
diff --git a/cpp-package/example/mlp_gpu.cpp b/cpp-package/example/mlp_gpu.cpp
index 531afbb29d..aee38ebad3 100644
--- a/cpp-package/example/mlp_gpu.cpp
+++ b/cpp-package/example/mlp_gpu.cpp
@@ -95,7 +95,7 @@ int main(int argc, char** argv) {
   opt->SetLRScheduler(std::move(lr_sch));
 
   // Create executor by binding parameters to the model
-  auto *exec = net.SimpleBind(ctx, args);
+  auto exec = net.SimpleBind(ctx, args);
   auto arg_names = net.ListArguments();
 
   // Create metrics
@@ -150,7 +150,6 @@ int main(int argc, char** argv) {
     LG << "Epoch[" << iter << "] Val-Accuracy=" << val_acc.Get();
   }
 
-  delete exec;
   MXNotifyShutdown();
   return 0;
 }
diff --git a/cpp-package/example/resnet.cpp b/cpp-package/example/resnet.cpp
index 03b3d72176..6ae9ce923c 100644
--- a/cpp-package/example/resnet.cpp
+++ b/cpp-package/example/resnet.cpp
@@ -187,7 +187,7 @@ int main(int argc, char const *argv[]) {
      ->SetParam("rescale_grad", 1.0 / batch_size)
      ->SetParam("clip_gradient", 10);
 
-  auto *exec = resnet.SimpleBind(Context::gpu(), args_map);
+  auto exec = resnet.SimpleBind(Context::gpu(), args_map);
   auto arg_names = resnet.ListArguments();
 
   for (int iter = 0; iter < max_epoch; ++iter) {
@@ -222,7 +222,6 @@ int main(int argc, char const *argv[]) {
     }
     LG << "Accuracy: " << acu.Get();
   }
-  delete exec;
   MXNotifyShutdown();
   return 0;
 }
diff --git a/cpp-package/example/test_score.cpp b/cpp-package/example/test_score.cpp
index 254a6d242f..f2fdcba576 100644
--- a/cpp-package/example/test_score.cpp
+++ b/cpp-package/example/test_score.cpp
@@ -97,7 +97,7 @@ int main(int argc, char** argv) {
   opt->SetLRScheduler(std::move(lr_sch));
 
   // Create executor by binding parameters to the model
-  auto *exec = net.SimpleBind(ctx, args);
+  auto exec = net.SimpleBind(ctx, args);
   auto arg_names = net.ListArguments();
 
   float score = 0;
@@ -143,7 +143,6 @@ int main(int argc, char** argv) {
     score = acc.Get();
   }
 
-  delete exec;
   MXNotifyShutdown();
   return score >= MIN_SCORE ? 0 : 1;
 }
diff --git a/cpp-package/include/mxnet-cpp/symbol.h b/cpp-package/include/mxnet-cpp/symbol.h
index 127ef156eb..1a3108cbb1 100644
--- a/cpp-package/include/mxnet-cpp/symbol.h
+++ b/cpp-package/include/mxnet-cpp/symbol.h
@@ -33,6 +33,7 @@
 #include "mxnet-cpp/base.h"
 #include "mxnet-cpp/ndarray.h"
 #include "mxnet-cpp/op_map.h"
+#include "mxnet-cpp/utils.h"
 
 namespace mxnet {
 namespace cpp {
@@ -231,7 +232,7 @@ class Symbol {
   * \param aux_map NDArray that stores the internal state in op
   * \return a new executor, which need to be free manually.
   */
-  Executor *SimpleBind(const Context &context,
+  std::unique_ptr<Executor> SimpleBind(const Context &context,
                        const std::map<std::string, NDArray> &args_map,
                        const std::map<std::string, NDArray> &arg_grad_store =
                            std::map<std::string, NDArray>(),
@@ -257,7 +258,7 @@ class Symbol {
   *shares state with shared_exec, and should not be used in parallel with it.
   * \return a new executor, which need to be free manually.
   */
-  Executor *Bind(const Context &context, const std::vector<NDArray> &arg_arrays,
+  std::unique_ptr<Executor> Bind(const Context &context, const std::vector<NDArray> &arg_arrays,
                  const std::vector<NDArray> &grad_arrays,
                  const std::vector<OpReqType> &grad_reqs,
                  const std::vector<NDArray> &aux_arrays,
@@ -267,7 +268,7 @@ class Symbol {
 
  private:
   std::shared_ptr<SymBlob> blob_ptr_;
-  static OpMap*& op_map();
+  static std::unique_ptr<OpMap>& op_map();
 };
 Symbol operator+(mx_float lhs, const Symbol &rhs);
 Symbol operator-(mx_float lhs, const Symbol &rhs);
diff --git a/cpp-package/include/mxnet-cpp/symbol.hpp b/cpp-package/include/mxnet-cpp/symbol.hpp
index 11590fad60..511860a796 100644
--- a/cpp-package/include/mxnet-cpp/symbol.hpp
+++ b/cpp-package/include/mxnet-cpp/symbol.hpp
@@ -33,13 +33,13 @@
 
 #include "dmlc/logging.h"
 #include "mxnet-cpp/symbol.h"
-
 #include "mxnet-cpp/op_suppl.h"
+#include "mxnet-cpp/utils.h"
 
 namespace mxnet {
 namespace cpp {
-inline OpMap*& Symbol::op_map() {
-  static OpMap* op_map_ = new OpMap();
+inline std::unique_ptr<OpMap>& Symbol::op_map() {
+  static std::unique_ptr<OpMap> op_map_(new OpMap());
   return op_map_;
 }
 inline Symbol::Symbol(SymbolHandle handle) {
@@ -324,7 +324,7 @@ inline void Symbol::InferArgsMap(
   }
 }
 
-inline Executor *Symbol::SimpleBind(
+inline std::unique_ptr<Executor> Symbol::SimpleBind(
     const Context &context, const std::map<std::string, NDArray> &args_map,
     const std::map<std::string, NDArray> &arg_grad_store,
     const std::map<std::string, OpReqType> &grad_req_type,
@@ -338,18 +338,18 @@ inline Executor *Symbol::SimpleBind(
                       &aux_arrays, args_map, arg_grad_store, grad_req_type,
                       aux_map);
 
-  return new Executor(*this, context, arg_arrays, grad_arrays, grad_reqs,
+  return std::make_unique<Executor>(*this, context, arg_arrays, grad_arrays, grad_reqs,
                       aux_arrays);
 }
 
-inline Executor *Symbol::Bind(const Context &context,
+inline std::unique_ptr<Executor> Symbol::Bind(const Context &context,
                        const std::vector<NDArray> &arg_arrays,
                        const std::vector<NDArray> &grad_arrays,
                        const std::vector<OpReqType> &grad_reqs,
                        const std::vector<NDArray> &aux_arrays,
                        const std::map<std::string, Context> &group_to_ctx,
                        Executor *shared_exec) {
-  return new Executor(*this, context, arg_arrays, grad_arrays, grad_reqs,
+  return std::make_unique<Executor>(*this, context, arg_arrays, grad_arrays, grad_reqs,
                       aux_arrays, group_to_ctx, shared_exec);
 }
 inline Symbol operator+(mx_float lhs, const Symbol &rhs) { return rhs + lhs; }
diff --git a/cpp-package/include/mxnet-cpp/utils.h b/cpp-package/include/mxnet-cpp/utils.h
new file mode 100644
index 0000000000..71acda73ec
--- /dev/null
+++ b/cpp-package/include/mxnet-cpp/utils.h
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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.
+ */
+
+/*!
+* \file utils.h
+* \brief utility functions
+* \author Pedro Larroy
+*/
+
+#ifndef MXNET_CPP_UTILS_H_
+#define MXNET_CPP_UTILS_H_
+
+namespace std {
+#if __cplusplus < 201402L
+template<typename T, typename... Args>
+std::unique_ptr<T> make_unique(Args&&... args) {
+    return std::unique_ptr<T>(new T(std::forward<Args>(args)...));
+}
+#endif
+}  // namespace std
+
+#endif  // MXNET_CPP_UTILS_H_


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services