pdxcodemonkey commented on a change in pull request #813:
URL: https://github.com/apache/geode-native/pull/813#discussion_r812123462



##########
File path: cppcache/integration-test/fw_dunit.cpp
##########
@@ -361,19 +355,48 @@ void Task::setTimeout(int seconds) {
   }
 }
 
-class TestProcess : virtual public dunit::Manager {
- private:
-  WorkerId m_sId;
-
+class TestProcess {
  public:
   TestProcess(const std::string &cmdline, uint32_t id)
-      : Manager(cmdline), m_sId(id) {}
+      : id_{id}, running_{false}, cmd_{cmdline} {}
 
-  WorkerId &getWorkerId() { return m_sId; }
+  WorkerId &getWorkerId() { return id_; }
+
+  void run() {
+    auto arguments = bpo::split_unix(cmd_);
+
+    std::string exe = arguments[0];
+    arguments.erase(arguments.begin());
+    process_ = bp::child(exe, bp::args = arguments);
+
+    process_.wait();
+    if (process_.exit_code() != 0) {
+      std::clog << "Worker " << id_.getIdName() << " exited with code "
+                << process_.exit_code() << std::endl;
+    }
+
+    running_ = false;
+  }
+
+  void start() {
+    running_ = true;
+    thread_ = std::thread{[this]() { run(); }};
+  }
+
+  void stop() {
+    if (thread_.joinable()) {
+      thread_.join();
+    }
+  }
+
+  bool running() const { return running_; }

Review comment:
       This will apparently still return true even after stop() is called - do 
we care?
   
   Can you confirm my understanding of the new shutdown logic?  It appears to 
me now that stop() waits for thread exit, so if a thread is stuck in a worker 
then all of the processes hang?  I'm not clear on how we guarantee proper 
termination here.  Not that the old logic was known for reliably terminating 
things, but....




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to