2 files changed, 116 insertions, 27 deletions

diff --git a/include/cru/common/SubProcess.h b/include/cru/common/SubProcess.h
index 0e05fd5a..6a3cc6e6 100644
--- a/include/cru/common/SubProcess.h
+++ b/include/cru/common/SubProcess.h
@@ -19,7 +19,7 @@ enum class PlatformSubProcessStatus {
   /**
    * @brief The process is failed to start.
    */
-  FailToStart,
+  FailedToStart,
   /**
    * @brief The process is running now.
    */
@@ -42,9 +42,16 @@ struct PlatformSubProcessStartInfo {
   std::vector<String> environments;
 };
 
+struct PlatformSubProcessExitResult {
+  int exit_code;
+};
+
 /**
  * @brief Base class of a platform process. It is one-time, which means it
  * starts and exits and can't start again.
+ * @remarks
+ * If an object of this class is destructed before the process ends, the process
+ * will be killed.
  */
 class PlatformSubProcessBase : public Object {
  public:
@@ -55,21 +62,59 @@ class PlatformSubProcessBase : public Object {
 
   /**
    * @brief Create and start a real process. If the program can't be created or
-   * start, an exception should be thrown.
+   * start, an exception will be thrown.
+   *
+   * @remarks This method will hold the data lock during running. It ensures
+   * after return, the process already tries to start and status is definitely
+   * not `Prepare` but `FailedToStart` or `Running` or `Exit`. So it is safe to
+   * use `GetStatus` to check whether the process tries to start, aka, this
+   * method has been called or not.
    */
   void Start();
+
   /**
-   * @brief Wait for the process to exit for at most `wait_time`.
+   * @brief Wait for the process to exit for at most `wait_time`. If the process
+   * already exits, it will return immediately. If the process has not started
+   * or failed to start, it will throw. Ensure `Start` is called and does not
+   * throw before call this.
+   *
+   * @remarks You may wish this returns bool to indicate whether it is timeout
+   * or the process exits. But no, even if it is timeout, the process may also
+   * exits due to task schedule.
    */
   void Wait(std::optional<std::chrono::milliseconds> wait_time);
-  PlatformSubProcessStatus GetStatus() const;
-  int GetExitCode() const;
+
+  /**
+   * @brief kill the process if it is running. If the process already exits,
+   * nothing will happen. If the process has not started or failed to start, it
+   * will throw. Ensure `Start` is called and does not throw before call this.
+   */
+  void Kill();
+
+  /**
+   * @brief Get the status of the process.
+   * 1. If the process has tried to start, aka `Start` is called, then this
+   * method must return `Running`, `FailedToStart`.
+   * 2. If it returns `Prepare`, `Start` is not called.
+   * 3. It does NOT guarantee that this return `Running` and the process is
+   * actually running. Because there might be a window that the process exits
+   * already but status is not updated.
+   */
+  PlatformSubProcessStatus GetStatus();
+
+  PlatformSubProcessExitResult GetExitResult();
   io::Stream* GetStdinStream();
   io::Stream* GetStdoutStream();
   io::Stream* GetStderrStream();
 
-  void Join();
-  void Detach();
+  bool IsAutoDeleteAfterProcessExit() const;
+
+  /**
+   * @brief If auto delete, this instance will delete it self after the process
+   * exits. The delete will be called at another thread. You must ensure this
+   * object is created by new.
+   */
+  void SetAutoDeleteAfterProcessExit(bool auto_delete);
 
  protected:
   /**
@@ -84,16 +129,26 @@ class PlatformSubProcessBase : public Object {
   virtual void PlatformCreateProcess() = 0;
 
   /**
-   * @brief Wait for the created process forever. After process exits, fill the
-   * result info of internal data.
+   * @brief Wait for the created process forever.
    *
-   * Implementation should wait for the real process forever.
+   * Implementation should wait for the real process forever, after that, fill
+   * internal data and returns exit result.
    *
-   * This method will be called on another thread. It will only called once
+   * This method will be called on another thread. It will only be called once
    * after a success call of `Start`. It is safe to write internal data in this
    * method because process lock will be hold and we won't write to internal.
    */
-  virtual void PlatformWaitForProcess() = 0;
+  virtual PlatformSubProcessExitResult PlatformWaitForProcess() = 0;
+
+  /**
+   * @brief Kill the process immediately.
+   *
+   * This method will be called on this thread. It will only be called once
+   * after a success call of `Start`. There will be a window that the window
+   * already exits but the status has not been updated and this is called. So
+   * handle this gracefully and write to internal data carefully.
+   */
+  virtual void PlatformKillProcess() = 0;
 
  private:
   auto CreateDataLockGuard() {
@@ -108,13 +163,7 @@ class PlatformSubProcessBase : public Object {
   PlatformSubProcessStartInfo start_info_;
 
  private:
-  enum class DisposeKind {
-    None,
-    Join,
-    Detach,
-  };
-
-  DisposeKind dispose_;
+  bool auto_delete_;
 
   PlatformSubProcessStatus status_;
 
diff --git a/src/common/SubProcess.cpp b/src/common/SubProcess.cpp
index f2bcf919..d69fb973 100644
--- a/src/common/SubProcess.cpp
+++ b/src/common/SubProcess.cpp
@@ -11,16 +11,12 @@ SubProcessException::~SubProcessException() {}
 
 PlatformSubProcessBase::PlatformSubProcessBase(
     const PlatformSubProcessStartInfo& start_info)
-    : dispose_(DisposeKind::None),
+    : auto_delete_(false),
       start_info_(start_info),
       data_lock_(data_mutex_, std::defer_lock),
       process_lock_(process_mutex_, std::defer_lock) {}
 
-PlatformSubProcessBase::~PlatformSubProcessBase() {
-  auto data_lock_guard = CreateDataLockGuard();
-  if (status_ )
-    switch (dispose_) { DisposeKind::Join: }
-}
+PlatformSubProcessBase::~PlatformSubProcessBase() {}
 
 void PlatformSubProcessBase::Start() {
   auto data_lock_guard = CreateDataLockGuard();
@@ -40,18 +36,62 @@ void PlatformSubProcessBase::Start() {
       auto data_lock_guard = CreateDataLockGuard();
       status_ = PlatformSubProcessStatus::Exited;
     });
+
+    process_thread_.detach();
   } catch (const std::exception& e) {
-    status_ = PlatformSubProcessStatus::FailToStart;
+    status_ = PlatformSubProcessStatus::FailedToStart;
   }
 }
 
 void PlatformSubProcessBase::Wait(
     std::optional<std::chrono::milliseconds> wait_time) {
+  auto status = GetStatus();
+
+  if (status == PlatformSubProcessStatus::Prepare) {
+    throw SubProcessException(
+        u"The process does not start. Can't wait for it.");
+  }
+
+  if (status == PlatformSubProcessStatus::FailedToStart) {
+    throw SubProcessException(
+        u"The process failed to start. Can't wait for it.");
+  }
+
+  if (status == PlatformSubProcessStatus::Exited) {
+    return;
+  }
+
   if (wait_time) {
-    process_lock_.try_lock_for(*wait_time);
+    auto locked = process_lock_.try_lock_for(*wait_time);
+    if (locked) {
+      process_lock_.unlock();
+    }
   } else {
     process_lock_.lock();
+    process_lock_.unlock();
   }
 }
 
+void PlatformSubProcessBase::Kill() {
+  auto status = GetStatus();
+
+  if (status == PlatformSubProcessStatus::Prepare) {
+    throw SubProcessException(u"The process does not start. Can't kill it.");
+  }
+
+  if (status == PlatformSubProcessStatus::FailedToStart) {
+    throw SubProcessException(u"The process failed to start. Can't kill it.");
+  }
+
+  if (status == PlatformSubProcessStatus::Exited) {
+    return;
+  }
+
+  PlatformKillProcess();
+}
+
+PlatformSubProcessStatus PlatformSubProcessBase::GetStatus() {
+  auto data_lock_guard = CreateDataLockGuard();
+  return status_;
+}
 }  // namespace cru