From 451982f9d3993df5ff60b76f82c4fcf605097493 Mon Sep 17 00:00:00 2001
From: Jesse Hallam <jesse.hallam@gmail.com>
Date: Thu, 5 Sep 2019 17:27:36 -0300
Subject: [PATCH] improved OnDeactivate handling (#11988)

* Deactivate plugins in parallel to improve shutdown time
* Give plugins at most 10s to handle OnDeactivate before forcefully terminating
---
 app/plugin_shutdown_test.go | 69 +++++++++++++++++++++++++++++++++++++
 plugin/environment.go       | 32 +++++++++++++++--
 2 files changed, 98 insertions(+), 3 deletions(-)
 create mode 100644 app/plugin_shutdown_test.go

diff --git a/app/plugin_shutdown_test.go b/app/plugin_shutdown_test.go
new file mode 100644
index 0000000000..2f99ca8231
--- /dev/null
+++ b/app/plugin_shutdown_test.go
@@ -0,0 +1,69 @@
+package app
+
+import (
+	"testing"
+	"time"
+)
+
+func TestPluginShutdownTest(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping test to verify forced shutdown of slow plugin")
+	}
+
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+
+	tearDown, _, _ := SetAppEnvironmentWithPlugins(t,
+		[]string{
+			`
+			package main
+
+			import (
+				"github.com/mattermost/mattermost-server/plugin"
+			)
+
+			type MyPlugin struct {
+				plugin.MattermostPlugin
+			}
+
+			func main() {
+				plugin.ClientMain(&MyPlugin{})
+			}
+			`,
+			`
+			package main
+
+			import (
+				"github.com/mattermost/mattermost-server/plugin"
+			)
+
+			type MyPlugin struct {
+				plugin.MattermostPlugin
+			}
+
+			func (p *MyPlugin) OnDeactivate() error {
+				c := make(chan bool)
+				<-c
+
+				return nil
+			}
+
+			func main() {
+				plugin.ClientMain(&MyPlugin{})
+			}
+		`,
+		}, th.App, th.App.NewPluginAPI)
+	defer tearDown()
+
+	done := make(chan bool)
+	go func() {
+		defer close(done)
+		th.App.ShutDownPlugins()
+	}()
+
+	select {
+	case <-done:
+	case <-time.After(15 * time.Second):
+		t.Fatal("failed to force plugin shutdown after 10 seconds")
+	}
+}
diff --git a/plugin/environment.go b/plugin/environment.go
index a6345579d6..e4410cf26a 100644
--- a/plugin/environment.go
+++ b/plugin/environment.go
@@ -288,16 +288,42 @@ func (env *Environment) Shutdown() {
 		env.pluginHealthCheckJob.Cancel()
 	}
 
+	var wg sync.WaitGroup
 	env.registeredPlugins.Range(func(key, value interface{}) bool {
 		rp := value.(*registeredPlugin)
 
-		if rp.supervisor != nil {
+		if rp.supervisor == nil {
+			return true
+		}
+
+		wg.Add(1)
+
+		done := make(chan bool)
+		go func() {
+			defer close(done)
 			if err := rp.supervisor.Hooks().OnDeactivate(); err != nil {
 				env.logger.Error("Plugin OnDeactivate() error", mlog.String("plugin_id", rp.BundleInfo.Manifest.Id), mlog.Err(err))
 			}
-			rp.supervisor.Shutdown()
-		}
+		}()
 
+		go func() {
+			defer wg.Done()
+
+			select {
+			case <-time.After(10 * time.Second):
+				env.logger.Warn("Plugin OnDeactivate() failed to complete in 10 seconds", mlog.String("plugin_id", rp.BundleInfo.Manifest.Id))
+			case <-done:
+			}
+
+			rp.supervisor.Shutdown()
+		}()
+
+		return true
+	})
+
+	wg.Wait()
+
+	env.registeredPlugins.Range(func(key, value interface{}) bool {
 		env.registeredPlugins.Delete(key)
 
 		return true