docker-archive / deploykit

A toolkit for creating and managing declarative, self-healing infrastructure.
Apache License 2.0
2.25k stars 262 forks source link

Avoid Deadlock on Failed Commands #825

Closed davefreitag closed 6 years ago

davefreitag commented 6 years ago

When StartWithHandlers() is called, the waitgroup gets incremented as part of the setup for stdout/stderr handlers. If the command fails to start, StartWithHandlers() returns prior to calling the handlers. This leaves the waitgroup in a permanently incremented state. If wg.Wait() is called, this will result in deadlock. To avoid this, we can call the handlers prior to returning the error.

This issue surfaced in the terraform provider when we lost access to NFS on manager nodes. The terraform commands that depended on that directory failed to start, and the 'terraform apply' routine ended up in deadlock. This prevented infrakit from recovering once NFS access had been restored.

Signed-off-by: Dave Freitag dcfreita@us.ibm.com

codecov[bot] commented 6 years ago

Codecov Report

Merging #825 into master will increase coverage by 0.74%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #825      +/-   ##
==========================================
+ Coverage   47.16%   47.91%   +0.74%     
==========================================
  Files          89       89              
  Lines        7944     7943       -1     
==========================================
+ Hits         3747     3806      +59     
+ Misses       3836     3776      -60     
  Partials      361      361
Impacted Files Coverage Δ
pkg/util/exec/exec.go 53.97% <100%> (+24.59%) :arrow_up:
pkg/provider/terraform/instance/apply.go 70.26% <0%> (+3.59%) :arrow_up:
pkg/rpc/mux/server.go 47.91% <0%> (+5.2%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f45bdcf...077bda4. Read the comment docs.

GordonTheTurtle commented 6 years ago

Please sign your commits following these rules: https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work The easiest way to do this is to amend the last commit:

$ git clone -b "hang" git@github.com:davefreitag/infrakit.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842354328968
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.