Add upgrade locking mechanism to core ui serving module

cmds/cmdbase/update.go

@@ -9,6 +9,7 @@ import (
 	"github.com/safing/portmaster/base/log"
 	"github.com/safing/portmaster/base/notifications"
 	"github.com/safing/portmaster/service"
+	"github.com/safing/portmaster/service/ui"
 	"github.com/safing/portmaster/service/updates"
 )
 
@@ -71,3 +72,4 @@ type updateDummyInstance struct{}
 func (udi *updateDummyInstance) Restart()                                    {}
 func (udi *updateDummyInstance) Shutdown()                                   {}
 func (udi *updateDummyInstance) Notifications() *notifications.Notifications { return nil }
+func (udi *updateDummyInstance) UI() *ui.UI                                  { return nil }

service/instance.go

@@ -438,6 +438,15 @@ func (i *Instance) BinaryUpdates() *updates.Updater {
 	return i.binaryUpdates
 }
 
+// GetBinaryUpdateFile returns the file path of a binary update file.
+func (i *Instance) GetBinaryUpdateFile(name string) (path string, err error) {
+	file, err := i.binaryUpdates.GetFile(name)
+	if err != nil {
+		return "", err
+	}
+	return file.Path(), nil
+}
+
 // IntelUpdates returns the updates module.
 func (i *Instance) IntelUpdates() *updates.Updater {
 	return i.intelUpdates

service/ui/api.go

@@ -2,35 +2,21 @@ package ui
 
 import (
 	"github.com/safing/portmaster/base/api"
-	"github.com/safing/portmaster/base/log"
 )
 
-func registerAPIEndpoints() error {
+func (ui *UI) registerAPIEndpoints() error {
 	return api.RegisterEndpoint(api.Endpoint{
 		Path:        "ui/reload",
 		Write:       api.PermitUser,
-		ActionFunc:  reloadUI,
+		ActionFunc:  ui.reloadUI,
 		Name:        "Reload UI Assets",
 		Description: "Removes all assets from the cache and reloads the current (possibly updated) version from disk when requested.",
 	})
 }
 
-func reloadUI(_ *api.Request) (msg string, err error) {
+func (ui *UI) reloadUI(_ *api.Request) (msg string, err error) {
-	appsLock.Lock()
-	defer appsLock.Unlock()
-
 	// Close all archives.
-	for id, archiveFS := range apps {
+	ui.CloseArchives()
-		err := archiveFS.Close()
-		if err != nil {
-			log.Warningf("ui: failed to close archive %s: %s", id, err)
-		}
-	}
-
-	// Reset index.
-	for key := range apps {
-		delete(apps, key)
-	}
 
 	return "all ui archives successfully reloaded", nil
 }

service/ui/module.go

@@ -1,27 +1,55 @@
 package ui
 
 import (
-	"errors"
 	"os"
 	"path/filepath"
+	"sync"
 	"sync/atomic"
 
 	"github.com/safing/portmaster/base/api"
 	"github.com/safing/portmaster/base/log"
 	"github.com/safing/portmaster/base/utils"
 	"github.com/safing/portmaster/service/mgr"
-	"github.com/safing/portmaster/service/updates"
+	"github.com/spkg/zipfs"
 )
 
-func prep() error {
+// UI serves the user interface files.
-	if err := registerAPIEndpoints(); err != nil {
+type UI struct {
-		return err
+	mgr      *mgr.Manager
-	}
+	instance instance
 
-	return registerRoutes()
+	archives     map[string]*zipfs.FileSystem
+	archivesLock sync.RWMutex
+
+	upgradeLock atomic.Bool
 }
 
-func start() error {
+// New returns a new UI module.
+func New(instance instance) (*UI, error) {
+	m := mgr.New("UI")
+	ui := &UI{
+		mgr:      m,
+		instance: instance,
+
+		archives: make(map[string]*zipfs.FileSystem),
+	}
+
+	if err := ui.registerAPIEndpoints(); err != nil {
+		return nil, err
+	}
+	if err := ui.registerRoutes(); err != nil {
+		return nil, err
+	}
+
+	return ui, nil
+}
+
+func (ui *UI) Manager() *mgr.Manager {
+	return ui.mgr
+}
+
+// Start starts the module.
+func (ui *UI) Start() error {
 	// Create a dummy directory to which processes change their working directory
 	// to. Currently this includes the App and the Notifier. The aim is protect
 	// all other directories and increase compatibility should any process want
@@ -30,7 +58,7 @@ func start() error {
 	// may seem dangerous, but proper permission on the parent directory provide
 	// (some) protection.
 	// Processes must _never_ read from this directory.
-	execDir := filepath.Join(module.instance.DataDir(), "exec")
+	execDir := filepath.Join(ui.instance.DataDir(), "exec")
 	err := os.MkdirAll(execDir, 0o0777) //nolint:gosec // This is intentional.
 	if err != nil {
 		log.Warningf("ui: failed to create safe exec dir: %s", err)
@@ -45,52 +73,67 @@ func start() error {
 	return nil
 }
 
-// UI serves the user interface files.
-type UI struct {
-	mgr *mgr.Manager
-
-	instance instance
-}
-
-func (ui *UI) Manager() *mgr.Manager {
-	return ui.mgr
-}
-
-// Start starts the module.
-func (ui *UI) Start() error {
-	return start()
-}
-
 // Stop stops the module.
 func (ui *UI) Stop() error {
 	return nil
 }
 
-var (
+func (ui *UI) getArchive(name string) (archive *zipfs.FileSystem, ok bool) {
-	shimLoaded atomic.Bool
+	ui.archivesLock.RLock()
-	module     *UI
+	defer ui.archivesLock.RUnlock()
-)
 
-// New returns a new UI module.
+	archive, ok = ui.archives[name]
-func New(instance instance) (*UI, error) {
+	return
-	if !shimLoaded.CompareAndSwap(false, true) {
+}
-		return nil, errors.New("only one instance allowed")
+
-	}
+func (ui *UI) setArchive(name string, archive *zipfs.FileSystem) {
-	m := mgr.New("UI")
+	ui.archivesLock.Lock()
-	module = &UI{
+	defer ui.archivesLock.Unlock()
-		mgr:      m,
+
-		instance: instance,
+	ui.archives[name] = archive
+}
+
+// CloseArchives closes all open archives.
+func (ui *UI) CloseArchives() {
+	if ui == nil {
+		return
 	}
 
-	if err := prep(); err != nil {
+	ui.archivesLock.Lock()
-		return nil, err
+	defer ui.archivesLock.Unlock()
+
+	// Close archives.
+	for _, archive := range ui.archives {
+		if err := archive.Close(); err != nil {
+			ui.mgr.Warn("failed to close ui archive", "err", err)
+		}
 	}
 
-	return module, nil
+	// Reset map.
+	clear(ui.archives)
+}
+
+// EnableUpgradeLock enables the upgrade lock and closes all open archives.
+func (ui *UI) EnableUpgradeLock() {
+	if ui == nil {
+		return
+	}
+
+	ui.upgradeLock.Store(true)
+	ui.CloseArchives()
+}
+
+// DisableUpgradeLock disables the upgrade lock.
+func (ui *UI) DisableUpgradeLock() {
+	if ui == nil {
+		return
+	}
+
+	ui.upgradeLock.Store(false)
 }
 
 type instance interface {
 	DataDir() string
 	API() *api.API
-	BinaryUpdates() *updates.Updater
+	GetBinaryUpdateFile(name string) (path string, err error)
 }

service/ui/serve.go

@@ -9,26 +9,19 @@ import (
 	"net/url"
 	"path/filepath"
 	"strings"
-	"sync"
 
 	"github.com/spkg/zipfs"
 
 	"github.com/safing/portmaster/base/api"
 	"github.com/safing/portmaster/base/log"
 	"github.com/safing/portmaster/base/utils"
-	"github.com/safing/portmaster/service/updates"
 )
 
-var (
+func (ui *UI) registerRoutes() error {
-	apps     = make(map[string]*zipfs.FileSystem)
-	appsLock sync.RWMutex
-)
-
-func registerRoutes() error {
 	// Server assets.
 	api.RegisterHandler(
 		"/assets/{resPath:[a-zA-Z0-9/\\._-]+}",
-		&archiveServer{defaultModuleName: "assets"},
+		&archiveServer{ui: ui, defaultModuleName: "assets"},
 	)
 
 	// Add slash to plain module namespaces.
@@ -38,7 +31,7 @@ func registerRoutes() error {
 	)
 
 	// Serve modules.
-	srv := &archiveServer{}
+	srv := &archiveServer{ui: ui}
 	api.RegisterHandler("/ui/modules/{moduleName:[a-z]+}/", srv)
 	api.RegisterHandler("/ui/modules/{moduleName:[a-z]+}/{resPath:[a-zA-Z0-9/\\._-]+}", srv)
 
@@ -52,6 +45,7 @@ func registerRoutes() error {
 }
 
 type archiveServer struct {
+	ui                *UI
 	defaultModuleName string
 }
 
@@ -82,39 +76,35 @@ func (bs *archiveServer) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		resPath = "index.html"
 	}
 
-	appsLock.RLock()
+	archiveFS, ok := bs.ui.getArchive(moduleName)
-	archiveFS, ok := apps[moduleName]
-	appsLock.RUnlock()
 	if ok {
 		ServeFileFromArchive(w, r, moduleName, archiveFS, resPath)
 		return
 	}
 
+	// Check if the upgrade lock is enabled.
+	if bs.ui.upgradeLock.Load() {
+		http.Error(w, "Resources locked, upgrade in progress.", http.StatusLocked)
+		return
+	}
+
 	// get file from update system
-	zipFile, err := module.instance.BinaryUpdates().GetFile(fmt.Sprintf("%s.zip", moduleName))
+	zipFile, err := bs.ui.instance.GetBinaryUpdateFile(fmt.Sprintf("%s.zip", moduleName))
 	if err != nil {
-		if errors.Is(err, updates.ErrNotFound) {
+		log.Tracef("ui: error loading module %s: %s", moduleName, err)
-			log.Tracef("ui: requested module %s does not exist", moduleName)
+		http.Error(w, err.Error(), http.StatusInternalServerError)
-			http.Error(w, err.Error(), http.StatusNotFound)
-		} else {
-			log.Tracef("ui: error loading module %s: %s", moduleName, err)
-			http.Error(w, err.Error(), http.StatusInternalServerError)
-		}
 		return
 	}
 
 	// Open archive from disk.
-	archiveFS, err = zipfs.New(zipFile.Path())
+	archiveFS, err = zipfs.New(zipFile)
 	if err != nil {
 		log.Tracef("ui: error prepping module %s: %s", moduleName, err)
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
 
-	appsLock.Lock()
+	bs.ui.setArchive(moduleName, archiveFS)
-	apps[moduleName] = archiveFS
-	appsLock.Unlock()
-
 	ServeFileFromArchive(w, r, moduleName, archiveFS, resPath)
 }
 

service/updates/module.go

@@ -20,6 +20,7 @@ import (
 	"github.com/safing/portmaster/base/utils"
 	"github.com/safing/portmaster/service/configure"
 	"github.com/safing/portmaster/service/mgr"
+	"github.com/safing/portmaster/service/ui"
 )
 
 const (
@@ -645,4 +646,5 @@ type instance interface {
 	Restart()
 	Shutdown()
 	Notifications() *notifications.Notifications
+	UI() *ui.UI
 }

service/updates/upgrade.go

@@ -24,6 +24,10 @@ func (u *Updater) upgrade(downloader *Downloader, ignoreVersion bool) error {
 		}
 	}
 
+	// Unload UI assets to be able to move files on Windows.
+	u.instance.UI().EnableUpgradeLock()
+	defer u.instance.UI().DisableUpgradeLock()
+
 	// Execute the upgrade.
 	upgradeError := u.upgradeMoveFiles(downloader)
 	if upgradeError == nil {