mattermost/plugin/supervisor.go
Jesse Hallam 835c0871a0
MM-11431: handle plugin deadlocks (#9167)
* ensure plugin is always shutdown

Once we call `.client.Client()` the plugin has started, and must be shut
down. `newSupervisor` sometimes returned with an error (and without a
reference to the supervisor), leaving the client running indefinitely.

* Clarify the documentation to explain that plugin hooks will not trigger until `OnActivate` returns successfully, and will stop triggering just before `OnDeactivate` is called.

* test for plugin deadlock

* plugin/environment.go: switch to sync.Map

From: https://golang.org/pkg/sync/#Map

> If a goroutine holds a RWMutex for reading and another goroutine might call Lock, no goroutine should expect to be able to acquire a read lock until the initial read lock is released. In particular, this prohibits recursive read locking. This is to ensure that the lock eventually becomes available; a blocked Lock call excludes new readers from acquiring the lock.

The previous `RWMutex` was not safe given that we effectively acquired read locks recursively (hook -> api -> hook). This worked up until we activated or deactivated plugins, tried to acquire a write lock, and the plugin used the API to effectively trigger another hook.

Switching to sync.Map avoids this by divesting the need to lock at all, avoiding the potential for a recursive lock in the first place.
2018-07-27 11:37:17 -04:00

109 lines
2.4 KiB
Go

// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
package plugin
import (
"fmt"
"os/exec"
"path/filepath"
"runtime"
"strings"
"time"
"github.com/hashicorp/go-plugin"
"github.com/mattermost/mattermost-server/mlog"
"github.com/mattermost/mattermost-server/model"
)
type supervisor struct {
pluginId string
client *plugin.Client
hooks Hooks
implemented [TotalHooksId]bool
}
func newSupervisor(pluginInfo *model.BundleInfo, parentLogger *mlog.Logger, apiImpl API) (retSupervisor *supervisor, retErr error) {
supervisor := supervisor{}
defer func() {
if retErr != nil {
supervisor.Shutdown()
}
}()
wrappedLogger := pluginInfo.WrapLogger(parentLogger)
hclogAdaptedLogger := &hclogAdapter{
wrappedLogger: wrappedLogger.WithCallerSkip(1),
extrasKey: "wrapped_extras",
}
pluginMap := map[string]plugin.Plugin{
"hooks": &hooksPlugin{
log: wrappedLogger,
apiImpl: apiImpl,
},
}
executable := filepath.Clean(filepath.Join(
".",
pluginInfo.Manifest.GetExecutableForRuntime(runtime.GOOS, runtime.GOARCH),
))
if strings.HasPrefix(executable, "..") {
return nil, fmt.Errorf("invalid backend executable")
}
executable = filepath.Join(pluginInfo.Path, executable)
supervisor.client = plugin.NewClient(&plugin.ClientConfig{
HandshakeConfig: handshake,
Plugins: pluginMap,
Cmd: exec.Command(executable),
SyncStdout: wrappedLogger.With(mlog.String("source", "plugin_stdout")).StdLogWriter(),
SyncStderr: wrappedLogger.With(mlog.String("source", "plugin_stderr")).StdLogWriter(),
Logger: hclogAdaptedLogger,
StartTimeout: time.Second * 3,
})
rpcClient, err := supervisor.client.Client()
if err != nil {
return nil, err
}
raw, err := rpcClient.Dispense("hooks")
if err != nil {
return nil, err
}
supervisor.hooks = raw.(Hooks)
if impl, err := supervisor.hooks.Implemented(); err != nil {
return nil, err
} else {
for _, hookName := range impl {
if hookId, ok := hookNameToId[hookName]; ok {
supervisor.implemented[hookId] = true
}
}
}
err = supervisor.Hooks().OnActivate()
if err != nil {
return nil, err
}
return &supervisor, nil
}
func (sup *supervisor) Shutdown() {
if sup.client != nil {
sup.client.Kill()
}
}
func (sup *supervisor) Hooks() Hooks {
return sup.hooks
}
func (sup *supervisor) Implements(hookId int) bool {
return sup.implemented[hookId]
}