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

(Bug Report) Shared WaitGroup in Exec plugin #1463

Closed
allen13 opened this issue Jul 7, 2016 · 5 comments
Closed

(Bug Report) Shared WaitGroup in Exec plugin #1463

allen13 opened this issue Jul 7, 2016 · 5 comments

Comments

@allen13
Copy link
Contributor

allen13 commented Jul 7, 2016

Bug report

The exec plugin gather function is relying on an instance shared WaitGroup which causes the gather function to hang indefinitely when run concurrently. This does not affect the current code base since the agent waits for gather to finish before kicking off a new one. I stumbled upon this while addressing an in-house requirement for gathers to kick off regardless of whether or not the previous one has finished.

@allen13
Copy link
Contributor Author

allen13 commented Jul 7, 2016

Submitted pull request 1464 to address the issue.

@phemmer
Copy link
Contributor

phemmer commented Jul 8, 2016

This does not affect the current code base since the agent waits for gather to finish before kicking off a new one

Then why is this an issue? Seems like a bug report for a bug that can't happen :-)

@allen13
Copy link
Contributor Author

allen13 commented Jul 8, 2016

Just addressing unsafe code. The waitGroup belongs in the gather function not in the struct instance.

@sparrc
Copy link
Contributor

sparrc commented Jul 14, 2016

@allen13 why is the code unsafe?

@allen13
Copy link
Contributor Author

allen13 commented Jul 14, 2016

@sparrc The wait group is unnecessarily sharing its state between Gather executions. Should a previous gather fail to close it's waitgroup, future Gathers will hang indefinitely. Creating a new waitGroup instance for each Gather guarantees isolation.

@sparrc sparrc closed this as completed in 1d9745e Jul 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants