MM-49353: Setup intermediate signal handlers for plugin shutdown (#28653)
There are several steps that a server runs through inside (*Server).Start after ch.initPlugins() till the signal handler is reached which handles the server shutdown procedure. The issue arises when the server is shutdown after ch.initPlugins() completes but before (*Server).Start finishes. In that case, the plugins are all started but they won't be shut down cleanly. To fix this edge-case, we set up an intermediate signal handler, which attaches itself as soon as ch.initPlugins is finished, allowing us to run the cleanup code in case the shutdown happens before (*Server).Start finishes. And when we do reach the main signal handler, we don't need this intermediate handler any more. So we just reset the handlers and use the main signal handler which takes care of shutting down the whole server. Note: This is still not 100% bug-proof because ch.initPlugins() will initialize _all_ plugins, and the shutdown can happen just after one plugin is initialized. To handle that case will require the need to set up signal handlers after every plugin init which feels like overkill to me. A sample flow diagram to visualize better: Edge-case server.Start() | ch.initPlugins() | <ctrl-c> | execute signal handler, os.Exit(1) Happy-path server.Start() | ch.initPlugins() | server.Start() finished | reset old signal handler | setup main signal handler | server runs on as usual until shutdown https://mattermost.atlassian.net/browse/MM-49353 ```release-note NONE ```
Этот коммит содержится в:
коммит произвёл
GitHub
родитель
7e912573da
Коммит
05cdb36886
@@ -4,9 +4,12 @@
|
||||
package app
|
||||
|
||||
import (
|
||||
"os"
|
||||
"os/signal"
|
||||
"runtime"
|
||||
"strings"
|
||||
"sync"
|
||||
"syscall"
|
||||
|
||||
"github.com/pkg/errors"
|
||||
|
||||
@@ -77,6 +80,7 @@ type Channels struct {
|
||||
postReminderMut sync.Mutex
|
||||
postReminderTask *model.ScheduledTask
|
||||
|
||||
interruptQuitChan chan struct{}
|
||||
scheduledPostMut sync.Mutex
|
||||
scheduledPostTask *model.ScheduledTask
|
||||
loginAttemptsMut sync.Mutex
|
||||
@@ -84,12 +88,13 @@ type Channels struct {
|
||||
|
||||
func NewChannels(s *Server) (*Channels, error) {
|
||||
ch := &Channels{
|
||||
srv: s,
|
||||
imageProxy: imageproxy.MakeImageProxy(s.platform, s.httpService, s.Log()),
|
||||
uploadLockMap: map[string]bool{},
|
||||
filestore: s.FileBackend(),
|
||||
exportFilestore: s.ExportFileBackend(),
|
||||
cfgSvc: s.Platform(),
|
||||
srv: s,
|
||||
imageProxy: imageproxy.MakeImageProxy(s.platform, s.httpService, s.Log()),
|
||||
uploadLockMap: map[string]bool{},
|
||||
filestore: s.FileBackend(),
|
||||
exportFilestore: s.ExportFileBackend(),
|
||||
cfgSvc: s.Platform(),
|
||||
interruptQuitChan: make(chan struct{}),
|
||||
}
|
||||
|
||||
// We are passing a partially filled Channels struct so that the enterprise
|
||||
@@ -159,6 +164,20 @@ func (ch *Channels) Start() error {
|
||||
ctx := request.EmptyContext(ch.srv.Log())
|
||||
ch.initPlugins(ctx, *ch.cfgSvc.Config().PluginSettings.Directory, *ch.cfgSvc.Config().PluginSettings.ClientDirectory)
|
||||
|
||||
interruptChan := make(chan os.Signal, 1)
|
||||
signal.Notify(interruptChan, syscall.SIGINT, syscall.SIGTERM)
|
||||
go func() {
|
||||
select {
|
||||
case <-interruptChan:
|
||||
if err := ch.Stop(); err != nil {
|
||||
ch.srv.Log().Warn("Error stopping channels", mlog.Err(err))
|
||||
}
|
||||
os.Exit(1)
|
||||
case <-ch.interruptQuitChan:
|
||||
return
|
||||
}
|
||||
}()
|
||||
|
||||
ch.AddConfigListener(func(prevCfg, cfg *model.Config) {
|
||||
// We compute the difference between configs
|
||||
// to ensure we don't re-init plugins unnecessarily.
|
||||
@@ -208,6 +227,8 @@ func (ch *Channels) Stop() error {
|
||||
}
|
||||
ch.dndTaskMut.Unlock()
|
||||
|
||||
close(ch.interruptQuitChan)
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
|
||||
@@ -109,6 +109,10 @@ func runServer(configStore *config.Store, interruptChan chan os.Signal) error {
|
||||
|
||||
notifyReady()
|
||||
|
||||
// Wiping off any signal handlers set before.
|
||||
// This may come from intermediary signal handlers requiring to clean
|
||||
// up resources before server.Start can finish.
|
||||
signal.Reset(syscall.SIGINT, syscall.SIGTERM)
|
||||
// wait for kill signal before attempting to gracefully shutdown
|
||||
// the running service
|
||||
signal.Notify(interruptChan, syscall.SIGINT, syscall.SIGTERM)
|
||||
|
||||
@@ -483,7 +483,7 @@ func (env *Environment) Shutdown() {
|
||||
env.TogglePluginHealthCheckJob(false)
|
||||
|
||||
var wg sync.WaitGroup
|
||||
env.registeredPlugins.Range(func(key, value any) bool {
|
||||
env.registeredPlugins.Range(func(_, value any) bool {
|
||||
rp := value.(registeredPlugin)
|
||||
|
||||
if rp.supervisor == nil || !env.IsActive(rp.BundleInfo.Manifest.Id) {
|
||||
|
||||
Ссылка в новой задаче
Block a user