From 361318f821fae8f7bd6e2a75687e33cff609f696 Mon Sep 17 00:00:00 2001 From: Hari Date: Tue, 16 Aug 2016 12:16:27 +0530 Subject: [PATCH] Delete Instance Task only in exit handler. We currently have a delete of instance task object after terminating the task as well. This could result in accessing a deleted instance task object (eg. when the small timeout value is configured or when task exit takes long). Changing this to delete only via exit handler. Conflicts: src/vnsw/agent/oper/instance_task.cc src/vnsw/agent/oper/instance_task.h Change-Id: I062400dae5ba580a44fdb4ce0c4c48994e9c1b95 closes-bug: #1612354 (cherry picked from commit f5b38972b8a1dcdf8de5cb84cc05192a8c146187) --- src/vnsw/agent/oper/instance_manager.cc | 2 ++ src/vnsw/agent/oper/instance_task.cc | 6 +++++- src/vnsw/agent/oper/instance_task.h | 3 +++ src/vnsw/agent/oper/libvirt_instance_adapter.h | 2 ++ 4 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/vnsw/agent/oper/instance_manager.cc b/src/vnsw/agent/oper/instance_manager.cc index 529b090a349..063ba356245 100644 --- a/src/vnsw/agent/oper/instance_manager.cc +++ b/src/vnsw/agent/oper/instance_manager.cc @@ -525,6 +525,8 @@ void InstanceManager::ScheduleNextTask(InstanceTaskQueue *task_queue) { if (delay >= (netns_timeout_ * 2)) { task->Terminate(); + if (task->IsSetup()) + return; } else { task->Stop(); return; diff --git a/src/vnsw/agent/oper/instance_task.cc b/src/vnsw/agent/oper/instance_task.cc index 99f7375aa02..ed21e35a260 100644 --- a/src/vnsw/agent/oper/instance_task.cc +++ b/src/vnsw/agent/oper/instance_task.cc @@ -15,7 +15,7 @@ InstanceTaskExecvp::InstanceTaskExecvp(const std::string &name, const std::string &cmd, int cmd_type, EventManager *evm) : name_(name), cmd_(cmd), input_(*(evm->io_service())), - pid_(0), cmd_type_(cmd_type), pipe_stdout_(false) { + setup_done_(false), pid_(0), cmd_type_(cmd_type), pipe_stdout_(false) { } void InstanceTaskExecvp::ReadData(const boost::system::error_code &ec, @@ -54,6 +54,9 @@ void InstanceTaskExecvp::Terminate() { kill(pid_, SIGKILL); } +bool InstanceTaskExecvp::IsSetup() { + return setup_done_; +} // If there is an error before the fork, task is set to "not running" // and "false" is returned to caller so that caller can take appropriate @@ -124,6 +127,7 @@ bool InstanceTaskExecvp::Run() { //the task again return false; } + setup_done_ = true; bzero(rx_buff_, sizeof(rx_buff_)); input_.async_read_some(boost::asio::buffer(rx_buff_, kBufLen), diff --git a/src/vnsw/agent/oper/instance_task.h b/src/vnsw/agent/oper/instance_task.h index f12602f445b..f908366e042 100644 --- a/src/vnsw/agent/oper/instance_task.h +++ b/src/vnsw/agent/oper/instance_task.h @@ -28,6 +28,7 @@ class InstanceTask { virtual bool Run() = 0; virtual void Stop() = 0; virtual void Terminate() = 0; + virtual bool IsSetup() = 0; // TODO reimplement instance_manager.cc to remove these two? virtual pid_t pid() const = 0; @@ -77,6 +78,7 @@ class InstanceTaskExecvp : public InstanceTask { bool Run(); void Stop(); void Terminate(); + bool IsSetup(); pid_t pid() const { return pid_; @@ -105,6 +107,7 @@ class InstanceTaskExecvp : public InstanceTask { const std::string name_; std::string cmd_; boost::asio::posix::stream_descriptor input_; + bool setup_done_; // indicates whether errors_ has a valid descriptor or not char rx_buff_[kBufLen]; pid_t pid_; diff --git a/src/vnsw/agent/oper/libvirt_instance_adapter.h b/src/vnsw/agent/oper/libvirt_instance_adapter.h index 9f36fd58618..a02bcea9169 100644 --- a/src/vnsw/agent/oper/libvirt_instance_adapter.h +++ b/src/vnsw/agent/oper/libvirt_instance_adapter.h @@ -38,6 +38,7 @@ class LibvirtInstanceAdapter : public InstanceManagerAdapter { bool Run(); void Stop() {} void Terminate() {} + bool IsSetup() { return true; } const std::string &cmd() const { static const std::string cmdstr = @@ -77,6 +78,7 @@ class LibvirtInstanceAdapter : public InstanceManagerAdapter { bool Run(); void Stop() {} void Terminate() {} + bool IsSetup() { return true; } const std::string &cmd() const { static const std::string cmdstr =