Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(process): proc status lost when streaming #3417

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
20 changes: 16 additions & 4 deletions src/process.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -349,8 +349,8 @@ namespace proc {
return _apps;
}

std::vector<ctx_t> &proc_t::get_apps() {
return _apps;
void proc_t::set_apps(std::vector<ctx_t> apps) {
_apps = std::move(apps);
}

// Gets application image from application list.
Expand All @@ -370,6 +370,14 @@ namespace proc {
return _app.name;
}

const boost::process::v1::environment& proc_t::get_env() const {
return _env;
}

void proc_t::set_env(boost::process::v1::environment env) {
_env = std::move(env);
}

proc_t::~proc_t() {
// It's not safe to call terminate() here because our proc_t is a static variable
// that may be destroyed after the Boost loggers have been destroyed. Instead,
Expand Down Expand Up @@ -695,10 +703,14 @@ namespace proc {
}

void refresh(const std::string &file_name) {
auto proc_opt = proc::parse(file_name);
const auto proc_opt = proc::parse(file_name);

if (proc_opt) {
proc = std::move(*proc_opt);
// Update the process object with the new environment and apps
// And, keep app running status.

proc.set_env(proc_opt->get_env());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After looking into this I have found another bug related to _env. It is used in terminate function, this means we have to preserve the _env until process terminates :/

The moment we call execute we should copy the _env to a member variable _app_env (name is debatable) and replace every usage of _env within execute and terminate with _app_env (except the first one where we copy it of course). Or we could store the new ENV to _current_env and the in execute copy it (_env = _current_env). I prefer the first renaming option, but whatever is easier is ok.

proc.set_apps(proc_opt->get_apps());
}
}
} // namespace proc
43 changes: 41 additions & 2 deletions src/process.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,49 @@ namespace proc {

~proc_t();

const std::vector<ctx_t> &get_apps() const;
std::vector<ctx_t> &get_apps();
/**
* @brief Get the list of applications.
* @return A constant reference to the vector of applications.
*/
const std::vector<ctx_t>& get_apps() const;

/**
* @brief Set the list of applications.
* @param apps The new list of applications.
* @note This will overwrite the existing list of applications.
* @see refresh(const std::string &file_name)
MiroKaku marked this conversation as resolved.
Show resolved Hide resolved
*/
void set_apps(std::vector<ctx_t> apps);

/**
* @brief Get the image path of the application with the given app_id.
* @param app_id The ID of the application.
* @return The image path of the application.
*/
std::string get_app_image(int app_id);

/**
* @brief Get the name of the last run application.
* @return The name of the last run application.
*/
std::string get_last_run_app_name();

/**
* @brief Get the environment variables.
* @return A constant reference to the environment variables.
*/
const boost::process::v1::environment& get_env() const;

/**
* @brief Set the environment variables.
* @param env The new environment variables.
* @note This will overwrite the existing environment variables.
*/
void set_env(boost::process::v1::environment env);

/**
* @brief Terminate the currently running process.
*/
void terminate();

private:
Expand Down
Loading