maxmustermann/pulumi
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Don't close eventChannel when panicking (#1891)

Browse source 
* Don't close eventChannel when panicking

The state of the system is completely unknown when panicking and in
general it's not safe to infer whether or not it is safe to close a
channel when in this tate.

* CR feedback

* Spelling
This commit is contained in:
Sean Gillespie 2018-09-05 17:50:48 -07:00 committed by GitHub
parent 193af7bda8
commit f83d32390c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
1 changed files with 10 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

14
pkg/backend/apply.go
View file

@ -75,10 +75,12 @@ func PreviewThenPrompt(ctx context.Context, kind apitype.UpdateKind, stack Stack
op UpdateOperation, apply Applier) (engine.ResourceChanges, error) {
// create a channel to hear about the update events from the engine. this will be used so that
// we can build up the diff display in case the user asks to see the details of the diff
// Note that eventsChannel is not closed in a `defer`. It is generally unsafe to do so, since defers run during
// panics and we can't know whether or not we were in the middle of writing to this channel when the panic occurred.
//
// Instead of using a `defer`, we manually close `eventsChannel` on every exit of this function.
eventsChannel := make(chan engine.Event)
defer func() {
close(eventsChannel)
}()
var events []engine.Event
go func() {
@ -98,6 +100,7 @@ func PreviewThenPrompt(ctx context.Context, kind apitype.UpdateKind, stack Stack
if !op.Opts.SkipPreview {
c, err := apply(ctx, kind, stack, op, true /*dryRun*/, false /*persist*/, eventsChannel)
if err != nil {
close(eventsChannel)
return c, err
}
changes = c
@ -105,11 +108,14 @@ func PreviewThenPrompt(ctx context.Context, kind apitype.UpdateKind, stack Stack
// If there are no changes, or we're auto-approving or just previewing, we can skip the confirmation prompt.
if op.Opts.AutoApprove || kind == apitype.PreviewUpdate {
close(eventsChannel)
return changes, nil
}
// Otherwise, ensure the user wants to proceed.
return changes, confirmBeforeUpdating(kind, stack, events, op.Opts)
err := confirmBeforeUpdating(kind, stack, events, op.Opts)
close(eventsChannel)
return changes, err
}
// confirmBeforeUpdating asks the user whether to proceed. A nil error means yes.