Revert "GODT-2039: bridge monitors bridge-gui via its PID."

This reverts commit 3b9a3aaad2.

internal/app/base/base.go

@@ -76,7 +76,6 @@ const (
 	flagRestart         = "restart"
 	FlagLauncher        = "launcher"
 	FlagNoWindow        = "no-window"
-	FlagParentPID       = "parent-pid"
 )
 
 type Base struct {
@@ -325,12 +324,6 @@ func (b *Base) NewApp(mainLoop func(*Base, *cli.Context) error) *cli.App {
 			Usage:  "The launcher to use to restart the application",
 			Hidden: true,
 		},
-		&cli.IntFlag{
-			Name:   FlagParentPID,
-			Usage:  "The PID of the process that started the application. Ignored if frontend is not gRPC",
-			Hidden: true,
-			Value:  -1,
-		},
 	}
 
 	return app

internal/app/base/restart.go

@@ -20,7 +20,6 @@ package base
 import (
 	"os"
 	"strconv"
-	"strings"
 
 	"github.com/sirupsen/logrus"
 	"golang.org/x/sys/execabs"
@@ -39,8 +38,6 @@ func (b *Base) restartApp(crash bool) error {
 		args = os.Args[1:]
 	}
 
-	args = removeFlagWithValue(args, FlagParentPID)
-
 	if b.launcher != "" {
 		args = forceLauncherFlag(args, b.launcher)
 	}
@@ -88,30 +85,6 @@ func incrementRestartFlag(args []string) []string {
 	return res
 }
 
-// removeFlagWithValue removes a flag that requires a value from a list of command line parameters.
-// The flag can be of the following form `-flag value`, `--flag value`, `-flag=value` or `--flags=value`.
-func removeFlagWithValue(argList []string, flag string) []string {
-	var result []string
-
-	for i := 0; i < len(argList); i++ {
-		arg := argList[i]
-		// "detect the parameter form "-flag value" or "--paramName value"
-		if (arg == "-"+flag) || (arg == "--"+flag) {
-			i++
-			continue
-		}
-
-		//  "detect the form "--flag=value" or "--flag=value"
-		if strings.HasPrefix(arg, "-"+flag+"=") || (strings.HasPrefix(arg, "--"+flag+"=")) {
-			continue
-		}
-
-		result = append(result, arg)
-	}
-
-	return result
-}
-
 // forceLauncherFlag  replace or add the launcher args with the one set in the app.
 func forceLauncherFlag(args []string, launcher string) []string {
 	res := append([]string{}, args...)

internal/app/base/restart_test.go

@@ -61,22 +61,3 @@ func TestVersionLessThan(t *testing.T) {
 	r.False(current.LessThan(current))
 	r.False(newer.LessThan(current))
 }
-
-func TestRemoveFlagWithValue(t *testing.T) {
-	tests := []struct {
-		argList  []string
-		flag     string
-		expected []string
-	}{
-		{[]string{}, "b", nil},
-		{[]string{"-a", "-b=value", "-c"}, "b", []string{"-a", "-c"}},
-		{[]string{"-a", "--b=value", "-c"}, "b", []string{"-a", "-c"}},
-		{[]string{"-a", "-b", "value", "-c"}, "b", []string{"-a", "-c"}},
-		{[]string{"-a", "--b", "value", "-c"}, "b", []string{"-a", "-c"}},
-		{[]string{"-a", "-B=value", "-c"}, "b", []string{"-a", "-B=value", "-c"}},
-	}
-
-	for _, tt := range tests {
-		require.Equal(t, removeFlagWithValue(tt.argList, tt.flag), tt.expected)
-	}
-}

internal/app/bridge/bridge.go

@@ -86,7 +86,6 @@ func main(b *base.Base, c *cli.Context) error { //nolint:funlen
 		b.Updater,
 		b,
 		b.Locations,
-		c.Int(base.FlagParentPID),
 	)
 
 	cache, cacheErr := loadMessageCache(b)

internal/frontend/bridge-gui/bridge-gui/main.cpp

@@ -257,10 +257,7 @@ void launchBridge(QStringList const &args)
     else
         app().log().debug(QString("Bridge executable path: %1").arg(QDir::toNativeSeparators(bridgeExePath)));
 
-    qint64 const pid = qApp->applicationPid();
+    overseer = std::make_unique<Overseer>(new ProcessMonitor(bridgeExePath, QStringList("--grpc") + args, nullptr), nullptr);
-    QStringList const  params =  QStringList { "--grpc", "--parent-pid", QString::number(pid) } + args ;
-    app().log().info(QString("Launching bridge process with command \"%1\" %2").arg(bridgeExePath, params.join(" ")));
-    overseer = std::make_unique<Overseer>(new ProcessMonitor(bridgeExePath, params , nullptr), nullptr);
     overseer->startWorker(true);
 }
 

internal/frontend/frontend.go

@@ -56,7 +56,6 @@ func New(
 	updater types.Updater,
 	restarter types.Restarter,
 	locations *locations.Locations,
-	parentPID int,
 ) Frontend {
 	switch frontendType {
 	case GRPC:
@@ -67,7 +66,6 @@ func New(
 			updater,
 			restarter,
 			locations,
-			parentPID,
 		)
 
 	case CLI:

internal/frontend/grpc/service.go

@@ -40,9 +40,6 @@ import (
 	"github.com/ProtonMail/proton-bridge/v2/pkg/keychain"
 	"github.com/ProtonMail/proton-bridge/v2/pkg/listener"
 	"github.com/ProtonMail/proton-bridge/v2/pkg/pmapi"
-	"github.com/bradenaw/juniper/xslices"
-	"github.com/elastic/go-sysinfo"
-	sysinfotypes "github.com/elastic/go-sysinfo/types"
 	"github.com/google/uuid"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -88,8 +85,6 @@ type Service struct { // nolint:structcheck
 	locations          *locations.Locations
 	token              string
 	pemCert            string
-	parentPID          int
-	parentPIDDoneCh    chan struct{}
 }
 
 // NewService returns a new instance of the service.
@@ -100,7 +95,6 @@ func NewService(
 	updater types.Updater,
 	restarter types.Restarter,
 	locations *locations.Locations,
-	parentPID int,
 ) *Service {
 	s := Service{
 		UnimplementedBridgeServer: UnimplementedBridgeServer{},
@@ -116,8 +110,6 @@ func NewService(
 		firstTimeAutostart: sync.Once{},
 		locations:          locations,
 		token:              uuid.NewString(),
-		parentPID:          parentPID,
-		parentPIDDoneCh:    make(chan struct{}),
 	}
 
 	// Initializing.Done is only called sync.Once. Please keep the increment
@@ -185,12 +177,6 @@ func (s *Service) Loop(b types.Bridger) error {
 	s.initAutostart()
 	s.startGRPCServer()
 
-	if s.parentPID < 0 {
-		s.log.Info("Not monitoring parent PID")
-	} else {
-		go s.monitorParentPID()
-	}
-
 	defer func() {
 		s.bridge.SetBool(settings.FirstStartGUIKey, false)
 	}()
@@ -534,36 +520,3 @@ func (s *Service) validateStreamServerToken(
 
 	return handler(srv, ss)
 }
-
-// monitorParentPID check at regular intervals that the parent process is still alive, and if not shuts down the server
-// and the applications.
-func (s *Service) monitorParentPID() {
-	s.log.Infof("Starting to monitor parent PID %v", s.parentPID)
-	ticker := time.NewTicker(5 * time.Second)
-
-	for {
-		select {
-		case <-ticker.C:
-			if s.parentPID < 0 {
-				continue
-			}
-
-			processes, err := sysinfo.Processes() // sysinfo.Process(pid) does not seem to work on Windows.
-			if err != nil {
-				s.log.Debug("Could not retrieve process list")
-				continue
-			}
-
-			if !xslices.Any(processes, func(p sysinfotypes.Process) bool { return p != nil && p.PID() == s.parentPID }) {
-				s.log.Info("Parent process does not exist anymore. Initiating shutdown")
-				go s.quit() // quit will write to the parentPIDDoneCh, so we launch a goroutine.
-			} else {
-				s.log.Tracef("Parent process %v is still alive", s.parentPID)
-			}
-
-		case <-s.parentPIDDoneCh:
-			s.log.Infof("Stopping process monitoring for PID %v", s.parentPID)
-			return
-		}
-	}
-}

internal/frontend/grpc/service_methods.go

@@ -95,19 +95,15 @@ func (s *Service) GuiReady(ctx context.Context, _ *emptypb.Empty) (*emptypb.Empt
 // Quit implement the Quit gRPC service call.
 func (s *Service) Quit(ctx context.Context, empty *emptypb.Empty) (*emptypb.Empty, error) {
 	s.log.Debug("Quit")
-	s.quit()
+	return &emptypb.Empty{}, s.quit()
-	return &emptypb.Empty{}, nil
 }
 
-func (s *Service) quit() {
+func (s *Service) quit() error {
 	// Windows is notably slow at Quitting. We do it in a goroutine to speed things up a bit.
 	go func() {
-		if s.parentPID >= 0 {
+		var err error
-			s.parentPIDDoneCh <- struct{}{}
-		}
-
 		if s.isStreamingEvents() {
-			if err := s.stopEventStream(); err != nil {
+			if err = s.stopEventStream(); err != nil {
 				s.log.WithError(err).Error("Quit failed.")
 			}
 		}
@@ -115,6 +111,8 @@ func (s *Service) quit() {
 		// The following call is launched as a goroutine, as it will wait for current calls to end, including this one.
 		s.grpcServer.GracefulStop()
 	}()
+
+	return nil
 }
 
 // Restart implement the Restart gRPC service call.

internal/frontend/grpc/service_stream.go

@@ -71,9 +71,8 @@ func (s *Service) RunEventStream(request *EventStreamRequest, server Bridge_RunE
 				return err
 			}
 		case <-server.Context().Done():
-			s.log.Info("Client closed the stream, initiating shutdown")
+			s.log.Debug("Client closed the stream, exiting")
-			s.quit()
+			return s.quit()
-			return nil
 		}
 	}
 }