From 6d69039c20d2002ba3f77b84faf23b9dcf128422 Mon Sep 17 00:00:00 2001 From: Patrick Pacher Date: Fri, 17 Jul 2020 11:07:46 +0200 Subject: [PATCH] Implement review comments --- cmds/portmaster-start/logs.go | 13 --------- cmds/portmaster-start/main.go | 16 ----------- cmds/portmaster-start/run.go | 7 ++--- cmds/portmaster-start/shutdown.go | 2 +- cmds/portmaster-start/update.go | 10 ++++++- updates/upgrader.go | 48 +++++++++++++++++++++++-------- 6 files changed, 49 insertions(+), 47 deletions(-) diff --git a/cmds/portmaster-start/logs.go b/cmds/portmaster-start/logs.go index 2a13e097..2e73f4ca 100644 --- a/cmds/portmaster-start/logs.go +++ b/cmds/portmaster-start/logs.go @@ -6,7 +6,6 @@ import ( "os" "path/filepath" "runtime" - "runtime/pprof" "time" "github.com/safing/portbase/container" @@ -113,18 +112,6 @@ func logControlError(cErr error) { fmt.Fprintln(errorFile, cErr.Error()) } -//nolint:deadcode,unused // TODO -func logControlStack() { - fp := getPmStartLogFile(".stack.log") - if fp == nil { - return - } - defer fp.Close() - - // write error and close - _ = pprof.Lookup("goroutine").WriteTo(fp, 2) -} - //nolint:deadcode,unused // false positive on linux, currently used by windows only func runAndLogControlError(wrappedFunc func(cmd *cobra.Command, args []string) error) func(cmd *cobra.Command, args []string) error { return func(cmd *cobra.Command, args []string) error { diff --git a/cmds/portmaster-start/main.go b/cmds/portmaster-start/main.go index 52ef057a..925dde68 100644 --- a/cmds/portmaster-start/main.go +++ b/cmds/portmaster-start/main.go @@ -75,15 +75,6 @@ func main() { // set meta info info.Set("Portmaster Start", "0.3.5", "AGPLv3", false) - // for debugging - // log.Start() - // log.SetLogLevel(log.TraceLevel) - // go func() { - // time.Sleep(3 * time.Second) - // pprof.Lookup("goroutine").WriteTo(os.Stdout, 2) - // os.Exit(1) - // }() - // catch interrupt for clean shutdown signalCh := make(chan os.Signal, 2) signal.Notify( @@ -103,13 +94,6 @@ func main() { os.Exit(0) }() - // for debugging windows service (no stdout/err) - // go func() { - // time.Sleep(10 * time.Second) - // // initiateShutdown(nil) - // // logControlStack() - // }() - // wait for signals for sig := range signalCh { if childIsRunning.IsSet() { diff --git a/cmds/portmaster-start/run.go b/cmds/portmaster-start/run.go index e9fa40f4..f4f26974 100644 --- a/cmds/portmaster-start/run.go +++ b/cmds/portmaster-start/run.go @@ -100,7 +100,7 @@ func getExecArgs(opts *Options, cmdArgs []string) []string { args := []string{"--data", dataDir} if stdinSignals { - args = append(args, "-input-signals") + args = append(args, "--input-signals") } args = append(args, cmdArgs...) return args @@ -110,7 +110,7 @@ func run(opts *Options, cmdArgs []string) (err error) { // set download option registry.Online = opts.AllowDownload - if isShutdown() { + if isShuttingDown() { return nil } @@ -328,8 +328,7 @@ func execute(opts *Options, args []string) (cont bool, err error) { // wait until shut down select { case <-finished: - case <-time.After(11 * time.Second): // portmaster core prints stack if not able to shutdown in 10 seconds - // kill + case <-time.After(3 * time.Minute): // portmaster core prints stack if not able to shutdown in 3 minutes, give it one more ... err = exc.Process.Kill() if err != nil { return false, fmt.Errorf("failed to kill %s: %s", opts.Identifier, err) diff --git a/cmds/portmaster-start/shutdown.go b/cmds/portmaster-start/shutdown.go index 20ffd2de..b42218a1 100644 --- a/cmds/portmaster-start/shutdown.go +++ b/cmds/portmaster-start/shutdown.go @@ -25,7 +25,7 @@ func initiateShutdown(err error) { } } -func isShutdown() bool { +func isShuttingDown() bool { select { case <-shuttingDown: return true diff --git a/cmds/portmaster-start/update.go b/cmds/portmaster-start/update.go index 45369af5..edb4c8f1 100644 --- a/cmds/portmaster-start/update.go +++ b/cmds/portmaster-start/update.go @@ -40,7 +40,15 @@ func downloadUpdates() error { } } - // ok, now we want logging. + // add updates that we require on all platforms. + registry.MandatoryUpdates = append( + registry.MandatoryUpdates, + "all/ui/modules/base.zip", + ) + + // logging is configured as a presistent pre-run method inherited from + // the root command but since we don't use run.Run() we need to start + // logging ourself. err := log.Start() if err != nil { fmt.Printf("failed to start logging: %s\n", err) diff --git a/updates/upgrader.go b/updates/upgrader.go index cb8100ee..254e42d8 100644 --- a/updates/upgrader.go +++ b/updates/upgrader.go @@ -148,30 +148,54 @@ func upgradePortmasterStart() error { } log.Infof("updates: upgraded %s", rootPmStartPath) + // TODO(ppacher): remove once we released a few more versions ... + warnOnIncorrectParentPath(filename) + + return nil +} + +func warnOnIncorrectParentPath(expectedFileName string) { // upgrade parent process, if it's portmaster-start parent, err := processInfo.NewProcess(int32(os.Getppid())) if err != nil { - return fmt.Errorf("could not get parent process for upgrade checks: %w", err) + log.Tracef("could not get parent process: %s", err) + return } parentName, err := parent.Name() if err != nil { - return fmt.Errorf("could not get parent process name for upgrade checks: %w", err) + log.Tracef("could not get parent process name: %s", err) + return } - if parentName != filename { - log.Tracef("updates: parent process does not seem to be portmaster-start, name is %s", parentName) - return nil + if parentName != expectedFileName { + log.Warningf("updates: parent process does not seem to be portmaster-start, name is %s", parentName) + + // TODO(ppacher): once we released a new installer and folks had time + // to update we should send a module warning/hint to the + // UI notifying the user that he's still using portmaster-control. + return } + parentPath, err := parent.Exe() if err != nil { - return fmt.Errorf("could not get parent process path for upgrade: %w", err) + log.Tracef("could not get parent process path: %s", err) + return } - err = upgradeFile(parentPath, pmCtrlUpdate) - if err != nil { - return err - } - log.Infof("updates: upgraded %s", parentPath) - return nil + absPath, err := filepath.Abs(parentPath) + if err != nil { + log.Tracef("could not get absolut parent process path: %s", err) + return + } + + root := filepath.Dir(registry.StorageDir().Path) + if !strings.HasPrefix(absPath, root) { + log.Warningf("detected unexpected path %s for portmaster-start", absPath) + + // TODO(ppacher): once we released a new installer and folks had time + // to update we should send a module warning/hint to the + // UI notifying the user that he's using portmaster-start + // from a wrong location. + } } func upgradeFile(fileToUpgrade string, file *updater.File) error {