From 91af528ff8935f5d1aa6dccb6562b3c63c9daca0 Mon Sep 17 00:00:00 2001
From: James Shubin <james@shubin.ca>
Date: Sun, 12 Mar 2017 13:47:22 -0400
Subject: [PATCH] pgraph: Move the quiesce done indicator to avoid deadlock

This avoids a deadlock on resource failure when retry==0. Without this
we would never exit. This adds a test in too!
---
 pgraph/actions.go         |  4 ++--
 test/shell/exec-fail.sh   |  9 +++++++++
 test/shell/exec-fail.yaml | 15 +++++++++++++++
 3 files changed, 26 insertions(+), 2 deletions(-)
 create mode 100755 test/shell/exec-fail.sh
 create mode 100644 test/shell/exec-fail.yaml

diff --git a/pgraph/actions.go b/pgraph/actions.go
index 76837283..ab82f85c 100644
--- a/pgraph/actions.go
+++ b/pgraph/actions.go
@@ -385,9 +385,9 @@ Loop:
 			// catch invalid rates
 			if v.Meta().Burst == 0 && !(v.Meta().Limit == rate.Inf) { // blocked
 				e := fmt.Errorf("%s[%s]: Permanently limited (rate != Inf, burst: 0)", v.Kind(), v.GetName())
-				v.SendEvent(event.EventExit, &SentinelErr{e})
 				ev.ACK() // ready for next message
 				v.Res.QuiesceGroup().Done()
+				v.SendEvent(event.EventExit, &SentinelErr{e})
 				continue
 			}
 
@@ -423,8 +423,8 @@ Loop:
 					log.Printf("%s[%s]: CheckApply errored: %v", v.Kind(), v.GetName(), e)
 					if retry == 0 {
 						// wrap the error in the sentinel
+						v.Res.QuiesceGroup().Done() // before the Wait that happens in SendEvent!
 						v.SendEvent(event.EventExit, &SentinelErr{e})
-						v.Res.QuiesceGroup().Done()
 						return
 					}
 					if retry > 0 { // don't decrement the -1
diff --git a/test/shell/exec-fail.sh b/test/shell/exec-fail.sh
new file mode 100755
index 00000000..adb41e1b
--- /dev/null
+++ b/test/shell/exec-fail.sh
@@ -0,0 +1,9 @@
+#!/bin/bash -e
+
+# should take a few seconds plus converged timeout, and test we don't hang!
+# TODO: should we return a different exit code if the resources fail?
+# TODO: should we be converged if one of the resources has permanently failed?
+$timeout --kill-after=20s 15s ./mgmt run --yaml exec-fail.yaml --converged-timeout=5 --no-watch --no-pgp --tmp-prefix &
+pid=$!
+wait $pid	# get exit status
+exit $?
diff --git a/test/shell/exec-fail.yaml b/test/shell/exec-fail.yaml
new file mode 100644
index 00000000..9c045554
--- /dev/null
+++ b/test/shell/exec-fail.yaml
@@ -0,0 +1,15 @@
+---
+graph: mygraph
+resources:
+  exec:
+  - name: exec1
+    cmd: false this should fail but not hang
+    shell: ''
+    timeout: 0
+    watchcmd: sleep 5s
+    watchshell: ''
+    ifcmd: ''
+    ifshell: ''
+    pollint: 0
+    state: present
+edges: []