Skip to content

Commit

Permalink
Delete Instance Task only in exit handler.
Browse files Browse the repository at this point in the history
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.

Change-Id: I062400dae5ba580a44fdb4ce0c4c48994e9c1b95
closes-bug: #1612354
(cherry picked from commit 361318f)
  • Loading branch information
haripk committed Aug 17, 2016
1 parent 98cf3ef commit 01c577c
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 1 deletion.
2 changes: 2 additions & 0 deletions src/vnsw/agent/oper/instance_manager.cc
Expand Up @@ -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;
Expand Down
6 changes: 5 additions & 1 deletion src/vnsw/agent/oper/instance_task.cc
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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),
Expand Down
3 changes: 3 additions & 0 deletions src/vnsw/agent/oper/instance_task.h
Expand Up @@ -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;
Expand Down Expand Up @@ -77,6 +78,7 @@ class InstanceTaskExecvp : public InstanceTask {
bool Run();
void Stop();
void Terminate();
bool IsSetup();

pid_t pid() const {
return pid_;
Expand Down Expand Up @@ -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_;
Expand Down
2 changes: 2 additions & 0 deletions src/vnsw/agent/oper/libvirt_instance_adapter.h
Expand Up @@ -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 =
Expand Down Expand Up @@ -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 =
Expand Down

0 comments on commit 01c577c

Please sign in to comment.