From ea98f9f4a9dc111670e31bee1491033df5d563e4 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Thu, 12 May 2022 10:57:23 +0530 Subject: [PATCH] MM-44045: Fix double plugin creation due to recursive plugin activation (#20165) The (*Environment).Activate method only guarded against multiple calls and therefore wasn't properly re-entrant. If a plugin calls an UpdateConfiguration in the OnActivate hook, it would again call this method, resulting in 2 plugin processes getting created. To prevent that from happening, we pre-emptively set the status to running just after the supervisor is created. I am not sure why starting the plugin normally from the server does not cause this because the root cause is the same and there should be 2 plugin processes created in the normal scenario as well. I have not spent time looking into that. But let me know if you want me to. Unit test: This might be a bit hard to test via a unit test. I think an e2e test maybe written in the playbooks test itself which counts the number of processes that the server spawns. https://mattermost.atlassian.net/browse/MM-44045 ```release-note NONE ``` --- plugin/environment.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/plugin/environment.go b/plugin/environment.go index 70c738733a..6e122662b4 100644 --- a/plugin/environment.go +++ b/plugin/environment.go @@ -274,6 +274,15 @@ func (env *Environment) Activate(id string) (manifest *model.Manifest, activated return nil, false, errors.Wrapf(err, "unable to start plugin: %v", id) } + // We pre-emptively set the state to running to prevent re-entrancy issues. + // The plugin's OnActivate hook can in-turn call UpdateConfiguration + // which again calls this method. This method is guarded against multiple calls, + // but fails if it is called recursively. + // + // Therefore, setting the state to running prevents this from happening, + // and in case there is an error, the defer clause will set the proper state anyways. + env.setPluginState(id, model.PluginStateRunning) + if err := sup.Hooks().OnActivate(); err != nil { sup.Shutdown() return nil, false, err