diff options
author | Jesse Hallam <jesse.hallam@gmail.com> | 2018-07-27 11:37:17 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-07-27 11:37:17 -0400 |
commit | 835c0871a01d9c0460ee34e22e9dfc53a69a2ac9 (patch) | |
tree | b37741460987ec74453445dd870bcae6437d09c0 /app/plugin_hooks_test.go | |
parent | 1d9c14485470d2399bb3ee578c11b0c50fb640c7 (diff) | |
download | chat-835c0871a01d9c0460ee34e22e9dfc53a69a2ac9.tar.gz chat-835c0871a01d9c0460ee34e22e9dfc53a69a2ac9.tar.bz2 chat-835c0871a01d9c0460ee34e22e9dfc53a69a2ac9.zip |
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.
Diffstat (limited to 'app/plugin_hooks_test.go')
-rw-r--r-- | app/plugin_hooks_test.go | 3 |
1 files changed, 1 insertions, 2 deletions
diff --git a/app/plugin_hooks_test.go b/app/plugin_hooks_test.go index 9846d628c..6e8434cab 100644 --- a/app/plugin_hooks_test.go +++ b/app/plugin_hooks_test.go @@ -43,6 +43,7 @@ func SetAppEnvironmentWithPlugins(t *testing.T, pluginCode []string, app *App, a env, err := plugin.NewEnvironment(apiFunc, pluginDir, webappPluginDir, app.Log) require.NoError(t, err) + app.Plugins = env for _, code := range pluginCode { pluginId := model.NewId() backend := filepath.Join(pluginDir, pluginId, "backend.exe") @@ -51,8 +52,6 @@ func SetAppEnvironmentWithPlugins(t *testing.T, pluginCode []string, app *App, a ioutil.WriteFile(filepath.Join(pluginDir, pluginId, "plugin.json"), []byte(`{"id": "`+pluginId+`", "backend": {"executable": "backend.exe"}}`), 0600) env.Activate(pluginId) } - - app.Plugins = env } func TestHookMessageWillBePosted(t *testing.T) { |