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.

1 files changed, 5 insertions, 2 deletions

diff --git a/plugin/hooks.go b/plugin/hooks.go
index 944909077..c191652e3 100644
--- a/plugin/hooks.go
+++ b/plugin/hooks.go
@@ -39,7 +39,9 @@ const (
 // A plugin only need implement the hooks it cares about. The MattermostPlugin provides some
 // default implementations for convenience but may be overridden.
 type Hooks interface {
-	// OnActivate is invoked when the plugin is activated.
+	// OnActivate is invoked when the plugin is activated. If an error is returned, the plugin
+	// will be terminated. The plugin will not receive hooks until after OnActivate returns
+	// without error.
 	OnActivate() error
 
 	// Implemented returns a list of hooks that are implemented by the plugin.
@@ -47,7 +49,8 @@ type Hooks interface {
 	Implemented() ([]string, error)
 
 	// OnDeactivate is invoked when the plugin is deactivated. This is the plugin's last chance to
-	// use the API, and the plugin will be terminated shortly after this invocation.
+	// use the API, and the plugin will be terminated shortly after this invocation. The plugin
+	// will stop receiving hooks just prior to this method being called.
 	OnDeactivate() error
 
 	// OnConfigurationChange is invoked when configuration changes may have been made.