From 6cc5123d314aae5746c5480e8b9b8345f0bb9af9 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 17 Feb 2017 08:32:21 -0800 Subject: [PATCH] vendor: update go-plugin This fixes some hanging issues seen by TF users, see the relevant commits in go-plugin. --- .../github.com/hashicorp/go-plugin/client.go | 62 +++++++++++++++---- .../hashicorp/go-plugin/rpc_server.go | 10 +++ vendor/vendor.json | 6 +- 3 files changed, 64 insertions(+), 14 deletions(-) diff --git a/vendor/github.com/hashicorp/go-plugin/client.go b/vendor/github.com/hashicorp/go-plugin/client.go index 3692736da..9f8a0f276 100644 --- a/vendor/github.com/hashicorp/go-plugin/client.go +++ b/vendor/github.com/hashicorp/go-plugin/client.go @@ -28,6 +28,7 @@ var Killed uint32 = 0 // This is a slice of the "managed" clients which are cleaned up when // calling Cleanup var managedClients = make([]*Client, 0, 5) +var managedClientsLock sync.Mutex // Error types var ( @@ -130,6 +131,7 @@ func CleanupClients() { // Kill all the managed clients in parallel and use a WaitGroup // to wait for them all to finish up. var wg sync.WaitGroup + managedClientsLock.Lock() for _, client := range managedClients { wg.Add(1) @@ -138,6 +140,7 @@ func CleanupClients() { wg.Done() }(client) } + managedClientsLock.Unlock() log.Println("[DEBUG] plugin: waiting for all plugin processes to complete...") wg.Wait() @@ -173,7 +176,9 @@ func NewClient(config *ClientConfig) (c *Client) { c = &Client{config: config} if config.Managed { + managedClientsLock.Lock() managedClients = append(managedClients, c) + managedClientsLock.Unlock() } return @@ -239,23 +244,58 @@ func (c *Client) Exited() bool { // // This method can safely be called multiple times. func (c *Client) Kill() { - if c.process == nil { + // Grab a lock to read some private fields. + c.l.Lock() + process := c.process + addr := c.address + doneCh := c.doneLogging + c.l.Unlock() + + // If there is no process, we never started anything. Nothing to kill. + if process == nil { return } - // Close the client to cleanly exit the process - client, err := c.Client() - if err == nil { - err = client.Close() - } - if err != nil { - // If something went wrong somewhere gracefully quitting the - // plugin, we just force kill it. - c.process.Kill() + // We need to check for address here. It is possible that the plugin + // started (process != nil) but has no address (addr == nil) if the + // plugin failed at startup. If we do have an address, we need to close + // the plugin net connections. + graceful := false + if addr != nil { + // Close the client to cleanly exit the process. + client, err := c.Client() + if err == nil { + err = client.Close() + + // If there is no error, then we attempt to wait for a graceful + // exit. If there was an error, we assume that graceful cleanup + // won't happen and just force kill. + graceful = err == nil + if err != nil { + // If there was an error just log it. We're going to force + // kill in a moment anyways. + log.Printf( + "[WARN] plugin: error closing client during Kill: %s", err) + } + } } + // If we're attempting a graceful exit, then we wait for a short period + // of time to allow that to happen. To wait for this we just wait on the + // doneCh which would be closed if the process exits. + if graceful { + select { + case <-doneCh: + return + case <-time.After(250 * time.Millisecond): + } + } + + // If graceful exiting failed, just kill it + process.Kill() + // Wait for the client to finish logging so we have a complete log - <-c.doneLogging + <-doneCh } // Starts the underlying subprocess, communicating with it to negotiate diff --git a/vendor/github.com/hashicorp/go-plugin/rpc_server.go b/vendor/github.com/hashicorp/go-plugin/rpc_server.go index 33601e207..3984dc891 100644 --- a/vendor/github.com/hashicorp/go-plugin/rpc_server.go +++ b/vendor/github.com/hashicorp/go-plugin/rpc_server.go @@ -7,12 +7,16 @@ import ( "log" "net" "net/rpc" + "sync" "github.com/hashicorp/yamux" ) // RPCServer listens for network connections and then dispenses interface // implementations over net/rpc. +// +// After setting the fields below, they shouldn't be read again directly +// from the structure which may be reading/writing them concurrently. type RPCServer struct { Plugins map[string]Plugin @@ -26,6 +30,8 @@ type RPCServer struct { // DoneCh should be set to a non-nil channel that will be closed // when the control requests the RPC server to end. DoneCh chan<- struct{} + + lock sync.Mutex } // Accept accepts connections on a listener and serves requests for @@ -102,8 +108,12 @@ func (s *RPCServer) ServeConn(conn io.ReadWriteCloser) { // doneCh to close which is listened to by the main process to cleanly // exit. func (s *RPCServer) done() { + s.lock.Lock() + defer s.lock.Unlock() + if s.DoneCh != nil { close(s.DoneCh) + s.DoneCh = nil } } diff --git a/vendor/vendor.json b/vendor/vendor.json index 87636bfd6..6ce517ccf 100644 --- a/vendor/vendor.json +++ b/vendor/vendor.json @@ -1680,10 +1680,10 @@ "revision": "d30f09973e19c1dfcd120b2d9c4f168e68d6b5d5" }, { - "checksumSHA1": "Jh6jdEjDeajnpEG8xlrLrnYw210=", + "checksumSHA1": "b0nQutPMJHeUmz4SjpreotAo6Yk=", "path": "github.com/hashicorp/go-plugin", - "revision": "8cf118f7a2f0c7ef1c82f66d4f6ac77c7e27dc12", - "revisionTime": "2016-06-08T02:21:58Z" + "revision": "f72692aebca2008343a9deb06ddb4b17f7051c15", + "revisionTime": "2017-02-17T16:27:05Z" }, { "checksumSHA1": "GBDE1KDl/7c5hlRPYRZ7+C0WQ0g=",