Implement review comments

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 {

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() {

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
+		case <-time.After(3 * time.Minute): // portmaster core prints stack if not able to shutdown in 3 minutes, give it one more ...
-			// kill
 			err = exc.Process.Kill()
 			if err != nil {
 				return false, fmt.Errorf("failed to kill %s: %s", opts.Identifier, err)

cmds/portmaster-start/shutdown.go

@@ -25,7 +25,7 @@ func initiateShutdown(err error) {
 	}
 }
 
-func isShutdown() bool {
+func isShuttingDown() bool {
 	select {
 	case <-shuttingDown:
 		return true

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)

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 {
+	if parentName != expectedFileName {
-		log.Tracef("updates: parent process does not seem to be portmaster-start, name is %s", parentName)
+		log.Warningf("updates: parent process does not seem to be portmaster-start, name is %s", parentName)
-		return nil
+
+		// 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 {