cmd/syncthing: Delay browser start until the GUI is ready (fixes #3619)

cmd/syncthing/gui.go

@@ -61,7 +61,7 @@ type apiService struct {
 	stop               chan struct{} // signals intentional stop
 	configChanged      chan struct{} // signals intentional listener close due to config change
 	started            chan string   // signals startup complete by sending the listener address, for testing only
-	startedOnce        bool          // the service has started successfully at least once
+	startedOnce        chan struct{} // the service has started successfully at least once
 
 	guiErrors logger.Recorder
 	systemLog logger.Recorder
@@ -129,6 +129,7 @@ func newAPIService(id protocol.DeviceID, cfg configIntf, httpsCertFile, httpsKey
 		systemConfigMut:    sync.NewMutex(),
 		stop:               make(chan struct{}),
 		configChanged:      make(chan struct{}),
+		startedOnce:        make(chan struct{}),
 		guiErrors:          errors,
 		systemLog:          systemLog,
 	}
@@ -202,20 +203,20 @@ func sendJSON(w http.ResponseWriter, jsonObject interface{}) {
 func (s *apiService) Serve() {
 	listener, err := s.getListener(s.cfg.GUI())
 	if err != nil {
-		if !s.startedOnce {
+		select {
+		case <-s.startedOnce:
+			// We let this be a loud user-visible warning as it may be the only
+			// indication they get that the GUI won't be available.
+			l.Warnln("Starting API/GUI:", err)
+			return
+
+		default:
 			// This is during initialization. A failure here should be fatal
 			// as there will be no way for the user to communicate with us
 			// otherwise anyway.
 			l.Fatalln("Starting API/GUI:", err)
 		}
-
-		// We let this be a loud user-visible warning as it may be the only
-		// indication they get that the GUI won't be available on startup.
-		l.Warnln("Starting API/GUI:", err)
-		return
 	}
-	s.startedOnce = true
-	defer listener.Close()
 
 	if listener == nil {
 		// Not much we can do here other than exit quickly. The supervisor
@@ -223,6 +224,8 @@ func (s *apiService) Serve() {
 		return
 	}
 
+	defer listener.Close()
+
 	// The GET handlers
 	getRestMux := http.NewServeMux()
 	getRestMux.HandleFunc("/rest/db/completion", s.getDBCompletion)              // device folder
@@ -339,6 +342,14 @@ func (s *apiService) Serve() {
 		s.started <- listener.Addr().String()
 	}
 
+	// Indicate successfull initial startup, to ourselves and to interested
+	// listeners (i.e. the thing that starts the browser).
+	select {
+	case <-s.startedOnce:
+	default:
+		close(s.startedOnce)
+	}
+
 	// Serve in the background
 
 	serveError := make(chan error, 1)

cmd/syncthing/main.go

@@ -947,6 +947,7 @@ func setupGUI(mainService *suture.Supervisor, cfg *config.Wrapper, m *model.Mode
 	if cfg.Options().StartBrowser && !runtimeOptions.noBrowser && !runtimeOptions.stRestarting {
 		// Can potentially block if the utility we are invoking doesn't
 		// fork, and just execs, hence keep it in it's own routine.
+		<-api.startedOnce
 		go openURL(guiCfg.URL())
 	}
 }