Merge pull request #29008 from cpuguy83/refcount_graphdriver

Refcount graphdriver plugins properly

daemon/graphdriver/plugin.go

@@ -22,10 +22,10 @@ func lookupPlugin(name, home string, opts []string, pg plugingetter.PluginGetter
 	if err != nil {
 		return nil, fmt.Errorf("Error looking up graphdriver plugin %s: %v", name, err)
 	}
-	return newPluginDriver(name, home, opts, pl.Client())
+	return newPluginDriver(name, home, opts, pl)
 }
 
-func newPluginDriver(name, home string, opts []string, c pluginClient) (Driver, error) {
-	proxy := &graphDriverProxy{name, c}
+func newPluginDriver(name, home string, opts []string, pl plugingetter.CompatPlugin) (Driver, error) {
+	proxy := &graphDriverProxy{name, pl.Client(), pl}
 	return proxy, proxy.Init(filepath.Join(home, name), opts)
 }

daemon/graphdriver/proxy.go

@@ -6,11 +6,13 @@ import (
 	"io"
 
 	"github.com/docker/docker/pkg/archive"
+	"github.com/docker/docker/pkg/plugingetter"
 )
 
 type graphDriverProxy struct {
 	name   string
 	client pluginClient
+	p      plugingetter.CompatPlugin
 }
 
 type graphDriverRequest struct {
@@ -35,6 +37,12 @@ type graphDriverInitRequest struct {
 }
 
 func (d *graphDriverProxy) Init(home string, opts []string) error {
+	if !d.p.IsV1() {
+		if cp, ok := d.p.(plugingetter.CountedPlugin); ok {
+			// always acquire here, it will be cleaned up on daemon shutdown
+			cp.Acquire()
+		}
+	}
 	args := &graphDriverInitRequest{
 		Home: home,
 		Opts: opts,
@@ -167,6 +175,13 @@ func (d *graphDriverProxy) GetMetadata(id string) (map[string]string, error) {
 }
 
 func (d *graphDriverProxy) Cleanup() error {
+	if !d.p.IsV1() {
+		if cp, ok := d.p.(plugingetter.CountedPlugin); ok {
+			// always release
+			defer cp.Release()
+		}
+	}
+
 	args := &graphDriverRequest{}
 	var ret graphDriverResponse
 	if err := d.client.Call("GraphDriver.Cleanup", args, &ret); err != nil {

pkg/plugingetter/getter.go

@@ -5,10 +5,10 @@ import "github.com/docker/docker/pkg/plugins"
 const (
 	// LOOKUP doesn't update RefCount
 	LOOKUP = 0
-	// CREATE increments RefCount
-	CREATE = 1
-	// REMOVE decrements RefCount
-	REMOVE = -1
+	// ACQUIRE increments RefCount
+	ACQUIRE = 1
+	// RELEASE decrements RefCount
+	RELEASE = -1
 )
 
 // CompatPlugin is a abstraction to handle both v2(new) and v1(legacy) plugins.
@@ -19,6 +19,13 @@ type CompatPlugin interface {
 	IsV1() bool
 }
 
+// CountedPlugin is a plugin which is reference counted.
+type CountedPlugin interface {
+	Acquire()
+	Release()
+	CompatPlugin
+}
+
 // PluginGetter is the interface implemented by Store
 type PluginGetter interface {
 	Get(name, capability string, mode int) (CompatPlugin, error)

plugin/v2/plugin.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/oci"
+	"github.com/docker/docker/pkg/plugingetter"
 	"github.com/docker/docker/pkg/plugins"
 	"github.com/docker/docker/pkg/system"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
@@ -294,6 +295,19 @@ func (p *Plugin) AddRefCount(count int) {
 	p.refCount += count
 }
 
+// Acquire increments the plugin's reference count
+// This should be followed up by `Release()` when the plugin is no longer in use.
+func (p *Plugin) Acquire() {
+	p.AddRefCount(plugingetter.ACQUIRE)
+}
+
+// Release decrements the plugin's reference count
+// This should only be called when the plugin is no longer in use, e.g. with
+// via `Acquire()` or getter.Get("name", "type", plugingetter.ACQUIRE)
+func (p *Plugin) Release() {
+	p.AddRefCount(plugingetter.RELEASE)
+}
+
 // InitSpec creates an OCI spec from the plugin's config.
 func (p *Plugin) InitSpec(s specs.Spec) (*specs.Spec, error) {
 	s.Root = specs.Root{

volume/drivers/extpoint.go

@@ -153,7 +153,7 @@ func CreateDriver(name string) (volume.Driver, error) {
 	if name == "" {
 		name = volume.DefaultDriverName
 	}
-	return lookup(name, getter.CREATE)
+	return lookup(name, getter.ACQUIRE)
 }
 
 // RemoveDriver returns a volume driver by its name and decrements RefCount..
@@ -162,7 +162,7 @@ func RemoveDriver(name string) (volume.Driver, error) {
 	if name == "" {
 		name = volume.DefaultDriverName
 	}
-	return lookup(name, getter.REMOVE)
+	return lookup(name, getter.RELEASE)
 }
 
 // GetDriverList returns list of volume drivers registered.