fix: serialize concurrent property updates

Concurrent commands for the same app could each read a setting, change it, and write it back, so all of them exited successfully while silently dropping changes made by the others. Updates to app settings are now serialized so every change is kept, and readers never see a partially written value.

docs/development/architecture.md

@@ -260,7 +260,7 @@ Dokku uses a file-based state system for simplicity and transparency.
 Plugin-specific configuration is stored in the property system:
 
 ```
-/var/lib/dokku/data/<plugin>/<app>/<property>
+/var/lib/dokku/config/<plugin>/<app>/<property>
 ```
 
 Properties are managed via helper functions:
@@ -277,6 +277,40 @@ common.PropertyWrite("git", appName, "deploy-branch", "main")
 common.PropertyGet("git", appName, "deploy-branch")
 ```
 
+Property writes for a given plugin and app are serialized by a lock on that app's property directory, and each write replaces the file atomically, so readers never see a partially written value. The list and map helpers, such as `PropertyListAdd` and `PropertyMapSet`, hold the lock while they read, change, and write a property.
+
+Code that reads a property and writes a value derived from it must hold the lock for the whole operation, or concurrent commands for the same app will overwrite each other's changes:
+
+```go
+// Go: update a single list property
+common.PropertyListUpdate("my-plugin", appName, "items", func(lines []string) ([]string, error) {
+    return append(lines, "new-item"), nil
+})
+
+// Go: read and write several properties together
+common.PropertyLock("my-plugin", appName, func(p *common.LockedProperties) error {
+    current := common.PropertyGet("my-plugin", appName, "current")
+    if err := p.Write("previous", current); err != nil {
+        return err
+    }
+    return p.Write("current", "new-value")
+})
+```
+
+```bash
+# Shell
+fn-my-plugin-rotate() {
+  declare desc="moves the current value to previous, expecting the caller to hold the my-plugin property lock"
+  declare APP="$1" VALUE="$2"
+  fn-plugin-property-write "my-plugin" "$APP" "previous" "$(fn-plugin-property-get "my-plugin" "$APP" "current")"
+  fn-plugin-property-write "my-plugin" "$APP" "current" "$VALUE"
+}
+
+fn-plugin-property-lock "my-plugin" "$APP" fn-my-plugin-rotate "$APP" "new-value"
+```
+
+Inside a `PropertyLock` callback, writes must go through the `LockedProperties` handle, as the public writers take the same lock. Shell callbacks may call the regular `fn-plugin-property-*` helpers.
+
 ### Per-App Data
 
 Application-specific data sometimes lives in `$DOKKU_ROOT/<app>/`:

plugins/checks/subcommands/disable

@@ -12,6 +12,12 @@ cmd-checks-disable() {
 
   verify_app_name "$APP"
   local PROCTYPES="${2:-_all_}"
+  fn-plugin-property-lock "checks" "$APP" fn-checks-disable "$APP" "$PROCTYPES"
+}
+
+fn-checks-disable() {
+  declare desc="disables zero-downtime checks for app/proctypes, expecting the caller to hold the checks property lock"
+  declare APP="$1" PROCTYPES="$2"
   local DOKKU_CHECKS_DISABLED=$(fn-plugin-property-get-default "checks" "$APP" "disabled" "")
   local DOKKU_CHECKS_SKIPPED=$(fn-plugin-property-get-default "checks" "$APP" "skipped" "")
 

plugins/checks/subcommands/enable

@@ -12,7 +12,12 @@ cmd-checks-enable() {
 
   verify_app_name "$APP"
   local PROCTYPES="${2:-_all_}"
+  fn-plugin-property-lock "checks" "$APP" fn-checks-enable "$APP" "$PROCTYPES"
+}
 
+fn-checks-enable() {
+  declare desc="enables zero-downtime checks for app/proctypes, expecting the caller to hold the checks property lock"
+  declare APP="$1" PROCTYPES="$2"
   local DOKKU_CHECKS_DISABLED=$(fn-plugin-property-get-default "checks" "$APP" "disabled" "")
   local DOKKU_CHECKS_SKIPPED=$(fn-plugin-property-get-default "checks" "$APP" "skipped" "")
 

plugins/checks/subcommands/skip

@@ -12,6 +12,12 @@ cmd-checks-skip() {
 
   verify_app_name "$APP"
   local PROCTYPES="${2:-_all_}"
+  fn-plugin-property-lock "checks" "$APP" fn-checks-skip "$APP" "$PROCTYPES"
+}
+
+fn-checks-skip() {
+  declare desc="skips zero-downtime checks for app/proctypes, expecting the caller to hold the checks property lock"
+  declare APP="$1" PROCTYPES="$2"
   local DOKKU_CHECKS_DISABLED=$(fn-plugin-property-get-default "checks" "$APP" "disabled" "")
   local DOKKU_CHECKS_SKIPPED=$(fn-plugin-property-get-default "checks" "$APP" "skipped" "")
 

plugins/common/lock.go

@@ -0,0 +1,60 @@
+package common
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+	"syscall"
+)
+
+// LockedDirectoriesEnvVar names the environment variable a caller holding a
+// directory lock exports to its child processes. It holds a newline-separated
+// list of directories, and LockDirectory runs its callback without locking a
+// directory found in that list, so a subprocess spawned while the lock is held
+// does not deadlock waiting on its own parent.
+const LockedDirectoriesEnvVar = "DOKKU_LOCKED_DIRECTORIES"
+
+// LockDirectory holds an exclusive flock on dir for the duration of fn. The
+// directory must already exist. The lock is advisory: it serializes callers
+// that go through LockDirectory, and does not block plain reads. Locks are
+// taken per open file description, so goroutines in a single process exclude
+// each other as well as separate processes do.
+//
+// A directory listed in DOKKU_LOCKED_DIRECTORIES is treated as already held by
+// a parent process, and fn runs without taking the lock again. LockDirectory is
+// not reentrant otherwise: fn must not call LockDirectory on the same
+// directory, as that would block forever.
+func LockDirectory(dir string, fn func() error) error {
+	dir = filepath.Clean(dir)
+	if isDirectoryLockHeld(dir) {
+		return fn()
+	}
+
+	file, err := os.Open(dir)
+	if err != nil {
+		return fmt.Errorf("Unable to open %s for locking: %w", dir, err)
+	}
+	defer file.Close()
+
+	if err := syscall.Flock(int(file.Fd()), syscall.LOCK_EX); err != nil {
+		return fmt.Errorf("Unable to lock %s: %w", dir, err)
+	}
+	defer syscall.Flock(int(file.Fd()), syscall.LOCK_UN)
+
+	return fn()
+}
+
+// isDirectoryLockHeld reports whether dir is listed in DOKKU_LOCKED_DIRECTORIES
+func isDirectoryLockHeld(dir string) bool {
+	for _, held := range strings.Split(os.Getenv(LockedDirectoriesEnvVar), "\n") {
+		held = strings.TrimSpace(held)
+		if held == "" {
+			continue
+		}
+		if filepath.Clean(held) == dir {
+			return true
+		}
+	}
+	return false
+}

plugins/common/properties.go

@@ -129,7 +129,7 @@ func PropertyGetAll(pluginName string, appName string) (map[string]string, error
 	}
 
 	for _, file := range files {
-		if file.IsDir() {
+		if file.IsDir() || strings.HasPrefix(file.Name(), ".") {
 			continue
 		}
 		property := file.Name()
@@ -159,7 +159,7 @@ func PropertyGetAllByPrefix(pluginName string, appName string, prefix string) (m
 	}
 
 	for _, file := range files {
-		if file.IsDir() {
+		if file.IsDir() || strings.HasPrefix(file.Name(), ".") {
 			continue
 		}
 		property := file.Name()
@@ -192,64 +192,49 @@ func PropertyGetDefault(pluginName, appName, property, defaultValue string) (val
 
 // PropertyListAdd adds a property to a list at an optionally specified index
 func PropertyListAdd(pluginName string, appName string, property string, value string, index int) error {
-	if err := propertyTouch(pluginName, appName, property); err != nil {
-		return err
-	}
-
-	scannedLines, err := PropertyListGet(pluginName, appName, property)
-	if err != nil {
-		return err
-	}
-
 	value = strings.TrimSpace(value)
+	return PropertyListUpdate(pluginName, appName, property, func(scannedLines []string) ([]string, error) {
+		var lines []string
+		for i, line := range scannedLines {
+			if index != 0 && i == (index-1) {
+				lines = append(lines, value)
+			}
+			lines = append(lines, line)
+		}
 
-	var lines []string
-	for i, line := range scannedLines {
-		if index != 0 && i == (index-1) {
+		if index == 0 || index > len(scannedLines) {
 			lines = append(lines, value)
 		}
-		lines = append(lines, line)
-	}
 
-	if index == 0 || index > len(scannedLines) {
-		lines = append(lines, value)
-	}
+		return lines, nil
+	})
+}
 
-	return PropertyListWrite(pluginName, appName, property, lines)
+// PropertyListUpdate reads a list property, passes it to fn and writes back the
+// list fn returns, holding the plugin/app property lock throughout so a
+// concurrent update cannot be lost between the read and the write. Nothing is
+// written when fn returns an error.
+func PropertyListUpdate(pluginName string, appName string, property string, fn func(lines []string) ([]string, error)) error {
+	return PropertyLock(pluginName, appName, func(p *LockedProperties) error {
+		lines, err := PropertyListGet(pluginName, appName, property)
+		if err != nil {
+			return err
+		}
+
+		lines, err = fn(lines)
+		if err != nil {
+			return err
+		}
+
+		return p.ListWrite(property, lines)
+	})
 }
 
 // PropertyListWrite completely rewrites a list property
 func PropertyListWrite(pluginName string, appName string, property string, values []string) error {
-	if err := propertyTouch(pluginName, appName, property); err != nil {
-		return err
-	}
-
-	propertyPath := getPropertyPath(pluginName, appName, property)
-	file, err := os.OpenFile(propertyPath, os.O_RDWR|os.O_TRUNC, 0600)
-	if err != nil {
-		return err
-	}
-
-	w := bufio.NewWriter(file)
-	for _, line := range values {
-		fmt.Fprintln(w, line)
-	}
-	if err = w.Flush(); err != nil {
-		return fmt.Errorf("Unable to write %s config value %s.%s: %s", pluginName, appName, property, err.Error())
-	}
-
-	if err := file.Close(); err != nil {
-		return fmt.Errorf("Unable to close %s config value %s.%s: %s", pluginName, appName, property, err.Error())
-	}
-
-	if err := SetPermissions(SetPermissionInput{
-		Filename: propertyPath,
-		Mode:     os.FileMode(0600),
-	}); err != nil {
-		return fmt.Errorf("Unable to set permissions for %s config value %s.%s: %s", pluginName, appName, property, err.Error())
-	}
-
-	return nil
+	return PropertyLock(pluginName, appName, func(p *LockedProperties) error {
+		return p.ListWrite(property, values)
+	})
 }
 
 // PropertyListGet returns a property list
@@ -350,71 +335,142 @@ func PropertyListGetByValue(pluginName string, appName string, property string,
 
 // PropertyListRemove removes a value from a property list
 func PropertyListRemove(pluginName string, appName string, property string, value string) error {
-	lines, err := PropertyListGet(pluginName, appName, property)
-	if err != nil {
-		return err
-	}
-
-	propertyPath := getPropertyPath(pluginName, appName, property)
-	file, err := os.OpenFile(propertyPath, os.O_RDWR|os.O_TRUNC, 0600)
-	if err != nil {
-		return err
-	}
-
-	found := false
-	w := bufio.NewWriter(file)
-	for _, line := range lines {
-		if line == value {
-			found = true
-			continue
-		}
-		fmt.Fprintln(w, line)
-	}
-	if err = w.Flush(); err != nil {
-		return fmt.Errorf("Unable to write %s config value %s.%s: %s", pluginName, appName, property, err.Error())
-	}
-
-	if err := file.Close(); err != nil {
-		return fmt.Errorf("Unable to close %s config value %s.%s: %s", pluginName, appName, property, err.Error())
-	}
-
-	if err := SetPermissions(SetPermissionInput{
-		Filename: propertyPath,
-		Mode:     os.FileMode(0600),
-	}); err != nil {
-		return fmt.Errorf("Unable to set permissions for %s config value %s.%s: %s", pluginName, appName, property, err.Error())
-	}
-
-	if !found {
-		return errors.New("Property not found, nothing was removed")
-	}
-
-	return nil
+	return propertyListRemoveMatching(pluginName, appName, property, func(line string) bool {
+		return line == value
+	})
 }
 
 // PropertyListRemoveByPrefix removes a value by prefix from a property list
 func PropertyListRemoveByPrefix(pluginName string, appName string, property string, prefix string) error {
-	lines, err := PropertyListGet(pluginName, appName, property)
-	if err != nil {
-		return err
+	return propertyListRemoveMatching(pluginName, appName, property, func(line string) bool {
+		return strings.HasPrefix(line, prefix)
+	})
+}
+
+// propertyListRemoveMatching drops every line of a list property that matches,
+// returning an error when nothing matched. A missing property is reported as
+// not found without taking the lock, so removing from an app with no
+// properties does not create its property directory.
+func propertyListRemoveMatching(pluginName string, appName string, property string, matches func(line string) bool) error {
+	errNotFound := errors.New("Property not found, nothing was removed")
+	if !PropertyExists(pluginName, appName, property) {
+		return errNotFound
 	}
 
-	propertyPath := getPropertyPath(pluginName, appName, property)
-	file, err := os.OpenFile(propertyPath, os.O_RDWR|os.O_TRUNC, 0600)
-	if err != nil {
-		return err
-	}
-
-	found := false
-	w := bufio.NewWriter(file)
-	for _, line := range lines {
-		if strings.HasPrefix(line, prefix) {
-			found = true
-			continue
+	return PropertyListUpdate(pluginName, appName, property, func(lines []string) ([]string, error) {
+		found := false
+		kept := []string{}
+		for _, line := range lines {
+			if matches(line) {
+				found = true
+				continue
+			}
+			kept = append(kept, line)
 		}
-		fmt.Fprintln(w, line)
+
+		if !found {
+			return nil, errNotFound
+		}
+		return kept, nil
+	})
+}
+
+// PropertyListSet sets a value within a property list at a specified index
+func PropertyListSet(pluginName string, appName string, property string, value string, index int) error {
+	value = strings.TrimSpace(value)
+	return PropertyListUpdate(pluginName, appName, property, func(scannedLines []string) ([]string, error) {
+		var lines []string
+		if index >= len(scannedLines) {
+			lines = append(lines, scannedLines...)
+			lines = append(lines, value)
+		} else {
+			for i, line := range scannedLines {
+				if i == index {
+					lines = append(lines, value)
+				} else {
+					lines = append(lines, line)
+				}
+			}
+		}
+
+		return lines, nil
+	})
+}
+
+// LockedProperties writes the properties of one plugin/app pair while the
+// caller holds that pair's property lock. It is handed to the PropertyLock
+// callback and is only valid until the callback returns.
+type LockedProperties struct {
+	pluginName string
+	appName    string
+}
+
+// Write writes a value for a property
+func (p *LockedProperties) Write(property string, value string) error {
+	return writePropertyFile(p.pluginName, p.appName, property, []byte(value))
+}
+
+// Delete deletes a property
+func (p *LockedProperties) Delete(property string) error {
+	return PropertyDelete(p.pluginName, p.appName, property)
+}
+
+// ListWrite completely rewrites a list property
+func (p *LockedProperties) ListWrite(property string, values []string) error {
+	var b strings.Builder
+	for _, line := range values {
+		b.WriteString(line)
+		b.WriteString("\n")
 	}
-	if err = w.Flush(); err != nil {
+	return writePropertyFile(p.pluginName, p.appName, property, []byte(b.String()))
+}
+
+// MapWrite persists a string→string map for a property as a single JSON file.
+// A nil map is persisted as "{}".
+func (p *LockedProperties) MapWrite(property string, m map[string]string) error {
+	if m == nil {
+		m = map[string]string{}
+	}
+
+	b, err := json.Marshal(m)
+	if err != nil {
+		return fmt.Errorf("Unable to marshal %s map property %s.%s: %w", p.pluginName, p.appName, property, err)
+	}
+
+	return writePropertyFile(p.pluginName, p.appName, property, b)
+}
+
+// PropertyLock runs fn while holding an exclusive lock on the properties of a
+// plugin/app pair, creating the property directory when it does not exist yet.
+// Use it to read properties and write values derived from them without a
+// concurrent writer slipping in between. Reads inside fn go through the normal
+// getters; writes must go through the LockedProperties handle, since the public
+// writers take this same lock and would block forever. Locks for other
+// plugin/app pairs may be taken inside fn.
+func PropertyLock(pluginName string, appName string, fn func(p *LockedProperties) error) error {
+	if err := makePluginAppPropertyPath(pluginName, appName); err != nil {
+		return fmt.Errorf("Unable to create %s config directory for %s: %s", pluginName, appName, err.Error())
+	}
+
+	return LockDirectory(getPluginAppPropertyPath(pluginName, appName), func() error {
+		return fn(&LockedProperties{pluginName: pluginName, appName: appName})
+	})
+}
+
+// writePropertyFile atomically replaces a property's contents. The data is
+// written to a hidden temporary file in the property directory and renamed
+// into place, so a concurrent reader sees either the old or the new contents
+// and never a truncated file.
+func writePropertyFile(pluginName string, appName string, property string, data []byte) error {
+	propertyPath := getPropertyPath(pluginName, appName, property)
+	file, err := os.CreateTemp(filepath.Dir(propertyPath), fmt.Sprintf(".%s.tmp-*", property))
+	if err != nil {
+		return fmt.Errorf("Unable to write %s config value %s.%s: %s", pluginName, appName, property, err.Error())
+	}
+	defer os.Remove(file.Name())
+
+	if _, err := file.Write(data); err != nil {
+		file.Close()
 		return fmt.Errorf("Unable to write %s config value %s.%s: %s", pluginName, appName, property, err.Error())
 	}
 
@@ -423,96 +479,24 @@ func PropertyListRemoveByPrefix(pluginName string, appName string, property stri
 	}
 
 	if err := SetPermissions(SetPermissionInput{
-		Filename: propertyPath,
+		Filename: file.Name(),
 		Mode:     os.FileMode(0600),
 	}); err != nil {
 		return fmt.Errorf("Unable to set permissions for %s config value %s.%s: %s", pluginName, appName, property, err.Error())
 	}
 
-	if !found {
-		return errors.New("Property not found, nothing was removed")
-	}
-
-	return nil
-}
-
-// PropertyListSet sets a value within a property list at a specified index
-func PropertyListSet(pluginName string, appName string, property string, value string, index int) error {
-	if err := propertyTouch(pluginName, appName, property); err != nil {
-		return err
-	}
-
-	scannedLines, err := PropertyListGet(pluginName, appName, property)
-	if err != nil {
-		return err
-	}
-
-	value = strings.TrimSpace(value)
-
-	var lines []string
-	if index >= len(scannedLines) {
-		lines = append(lines, scannedLines...)
-		lines = append(lines, value)
-	} else {
-		for i, line := range scannedLines {
-			if i == index {
-				lines = append(lines, value)
-			} else {
-				lines = append(lines, line)
-			}
-		}
-	}
-
-	return PropertyListWrite(pluginName, appName, property, lines)
-}
-
-// propertyTouch ensures a given application property file exists
-func propertyTouch(pluginName string, appName string, property string) error {
-	if err := makePluginAppPropertyPath(pluginName, appName); err != nil {
-		return fmt.Errorf("Unable to create %s config directory for %s: %s", pluginName, appName, err.Error())
-	}
-
-	propertyPath := getPropertyPath(pluginName, appName, property)
-	if PropertyExists(pluginName, appName, property) {
-		return nil
-	}
-
-	file, err := os.Create(propertyPath)
-	if err != nil {
+	if err := os.Rename(file.Name(), propertyPath); err != nil {
 		return fmt.Errorf("Unable to write %s config value %s.%s: %s", pluginName, appName, property, err.Error())
 	}
-	defer file.Close()
 
 	return nil
 }
 
 // PropertyWrite writes a value for a given application property
 func PropertyWrite(pluginName string, appName string, property string, value string) error {
-	if err := propertyTouch(pluginName, appName, property); err != nil {
-		return err
-	}
-
-	propertyPath := getPropertyPath(pluginName, appName, property)
-	file, err := os.Create(propertyPath)
-	if err != nil {
-		return fmt.Errorf("Unable to write %s config value %s.%s: %s", pluginName, appName, property, err.Error())
-	}
-	defer file.Close()
-
-	fmt.Fprint(file, value)
-
-	if err := file.Close(); err != nil {
-		return fmt.Errorf("Unable to close %s config value %s.%s: %s", pluginName, appName, property, err.Error())
-	}
-
-	if err := SetPermissions(SetPermissionInput{
-		Filename: propertyPath,
-		Mode:     os.FileMode(0600),
-	}); err != nil {
-		return fmt.Errorf("Unable to set permissions for %s config value %s.%s: %s", pluginName, appName, property, err.Error())
-	}
-
-	return nil
+	return PropertyLock(pluginName, appName, func(p *LockedProperties) error {
+		return p.Write(property, value)
+	})
 }
 
 // PropertyMapWrite persists a string→string map for a given property as a single
@@ -520,32 +504,9 @@ func PropertyWrite(pluginName string, appName string, property string, value str
 // bytes (including '/' or '\n') because they live inside file content rather
 // than in the on-disk filename. A nil map is persisted as "{}".
 func PropertyMapWrite(pluginName string, appName string, property string, m map[string]string) error {
-	if err := propertyTouch(pluginName, appName, property); err != nil {
-		return err
-	}
-
-	if m == nil {
-		m = map[string]string{}
-	}
-
-	b, err := json.Marshal(m)
-	if err != nil {
-		return fmt.Errorf("Unable to marshal %s map property %s.%s: %w", pluginName, appName, property, err)
-	}
-
-	propertyPath := getPropertyPath(pluginName, appName, property)
-	if err := os.WriteFile(propertyPath, b, 0600); err != nil {
-		return fmt.Errorf("Unable to write %s map property %s.%s: %w", pluginName, appName, property, err)
-	}
-
-	if err := SetPermissions(SetPermissionInput{
-		Filename: propertyPath,
-		Mode:     os.FileMode(0600),
-	}); err != nil {
-		return fmt.Errorf("Unable to set permissions for %s map property %s.%s: %w", pluginName, appName, property, err)
-	}
-
-	return nil
+	return PropertyLock(pluginName, appName, func(p *LockedProperties) error {
+		return p.MapWrite(property, m)
+	})
 }
 
 // PropertyMapGet returns the string→string map persisted for a property. A
@@ -573,12 +534,14 @@ func PropertyMapGet(pluginName string, appName string, property string) (map[str
 
 // PropertyMapSet reads the current map, sets one key, and writes the map back.
 func PropertyMapSet(pluginName string, appName string, property string, key string, value string) error {
-	m, err := PropertyMapGet(pluginName, appName, property)
-	if err != nil {
-		return err
-	}
-	m[key] = value
-	return PropertyMapWrite(pluginName, appName, property, m)
+	return PropertyLock(pluginName, appName, func(p *LockedProperties) error {
+		m, err := PropertyMapGet(pluginName, appName, property)
+		if err != nil {
+			return err
+		}
+		m[key] = value
+		return p.MapWrite(property, m)
+	})
 }
 
 // PropertyMapDelete reads the current map, removes one key, and writes the map
@@ -587,15 +550,17 @@ func PropertyMapDelete(pluginName string, appName string, property string, key s
 	if !PropertyExists(pluginName, appName, property) {
 		return nil
 	}
-	m, err := PropertyMapGet(pluginName, appName, property)
-	if err != nil {
-		return err
-	}
-	if _, ok := m[key]; !ok {
-		return nil
-	}
-	delete(m, key)
-	return PropertyMapWrite(pluginName, appName, property, m)
+	return PropertyLock(pluginName, appName, func(p *LockedProperties) error {
+		m, err := PropertyMapGet(pluginName, appName, property)
+		if err != nil {
+			return err
+		}
+		if _, ok := m[key]; !ok {
+			return nil
+		}
+		delete(m, key)
+		return p.MapWrite(property, m)
+	})
 }
 
 // PropertyMapLength returns the number of entries in the map persisted for a

plugins/common/properties_test.go

@@ -1,9 +1,14 @@
 package common
 
 import (
+	"errors"
+	"fmt"
 	"os"
 	"path/filepath"
+	"strconv"
+	"sync"
 	"testing"
+	"time"
 
 	. "github.com/onsi/gomega"
 )
@@ -168,3 +173,172 @@ func TestPropertyMapMalformedJSONReturnsError(t *testing.T) {
 	_, err := PropertyMapGet(propertyTestPlugin, propertyTestApp, propertyTestName)
 	Expect(err).To(HaveOccurred())
 }
+
+func TestPropertyListAddConcurrent(t *testing.T) {
+	setupPropertyMapTest(t)
+
+	Expect(runConcurrently(50, func(i int) error {
+		return PropertyListAdd(propertyTestPlugin, propertyTestApp, "list", fmt.Sprintf("value-%d", i), 0)
+	})).To(Succeed())
+
+	lines, err := PropertyListGet(propertyTestPlugin, propertyTestApp, "list")
+	Expect(err).NotTo(HaveOccurred())
+	Expect(lines).To(HaveLen(50))
+	for i := 0; i < 50; i++ {
+		Expect(lines).To(ContainElement(fmt.Sprintf("value-%d", i)))
+	}
+}
+
+func TestPropertyListRemoveConcurrent(t *testing.T) {
+	setupPropertyMapTest(t)
+
+	values := []string{}
+	for i := 0; i < 50; i++ {
+		values = append(values, fmt.Sprintf("value-%d", i))
+	}
+	Expect(PropertyListWrite(propertyTestPlugin, propertyTestApp, "list", values)).To(Succeed())
+
+	Expect(runConcurrently(len(values), func(i int) error {
+		return PropertyListRemove(propertyTestPlugin, propertyTestApp, "list", values[i])
+	})).To(Succeed())
+
+	lines, err := PropertyListGet(propertyTestPlugin, propertyTestApp, "list")
+	Expect(err).NotTo(HaveOccurred())
+	Expect(lines).To(BeEmpty())
+}
+
+func TestPropertyListRemoveMissingDoesNotCreateDirectory(t *testing.T) {
+	setupPropertyMapTest(t)
+
+	Expect(PropertyListRemove(propertyTestPlugin, "missing-app", "list", "value")).NotTo(Succeed())
+	Expect(PropertyListRemoveByPrefix(propertyTestPlugin, "missing-app", "list", "value")).NotTo(Succeed())
+	Expect(PropertyMapDelete(propertyTestPlugin, "missing-app", "map", "key")).To(Succeed())
+	Expect(DirectoryExists(getPluginAppPropertyPath(propertyTestPlugin, "missing-app"))).To(BeFalse())
+}
+
+func TestPropertyListRemoveNotFoundLeavesList(t *testing.T) {
+	setupPropertyMapTest(t)
+
+	Expect(PropertyListWrite(propertyTestPlugin, propertyTestApp, "list", []string{"a", "b"})).To(Succeed())
+	Expect(PropertyListRemove(propertyTestPlugin, propertyTestApp, "list", "c")).To(MatchError("Property not found, nothing was removed"))
+
+	lines, err := PropertyListGet(propertyTestPlugin, propertyTestApp, "list")
+	Expect(err).NotTo(HaveOccurred())
+	Expect(lines).To(Equal([]string{"a", "b"}))
+}
+
+func TestPropertyListUpdateErrorLeavesListUnchanged(t *testing.T) {
+	setupPropertyMapTest(t)
+
+	Expect(PropertyListWrite(propertyTestPlugin, propertyTestApp, "list", []string{"a", "b"})).To(Succeed())
+	err := PropertyListUpdate(propertyTestPlugin, propertyTestApp, "list", func(lines []string) ([]string, error) {
+		return nil, errors.New("boom")
+	})
+	Expect(err).To(MatchError("boom"))
+
+	lines, err := PropertyListGet(propertyTestPlugin, propertyTestApp, "list")
+	Expect(err).NotTo(HaveOccurred())
+	Expect(lines).To(Equal([]string{"a", "b"}))
+}
+
+func TestPropertyMapSetConcurrent(t *testing.T) {
+	setupPropertyMapTest(t)
+
+	Expect(runConcurrently(50, func(i int) error {
+		return PropertyMapSet(propertyTestPlugin, propertyTestApp, propertyTestName, fmt.Sprintf("key-%d", i), "value")
+	})).To(Succeed())
+
+	got, err := PropertyMapGet(propertyTestPlugin, propertyTestApp, propertyTestName)
+	Expect(err).NotTo(HaveOccurred())
+	Expect(got).To(HaveLen(50))
+}
+
+func TestPropertyLockSerializesReadModifyWrite(t *testing.T) {
+	setupPropertyMapTest(t)
+
+	Expect(runConcurrently(50, func(i int) error {
+		return PropertyLock(propertyTestPlugin, propertyTestApp, func(p *LockedProperties) error {
+			current, err := strconv.Atoi(PropertyGetDefault(propertyTestPlugin, propertyTestApp, "counter", "0"))
+			if err != nil {
+				return err
+			}
+			return p.Write("counter", strconv.Itoa(current+1))
+		})
+	})).To(Succeed())
+
+	Expect(PropertyGet(propertyTestPlugin, propertyTestApp, "counter")).To(Equal("50"))
+}
+
+func TestLockDirectorySkipsDirectoriesHeldByParent(t *testing.T) {
+	setupPropertyMapTest(t)
+
+	dir := getPluginAppPropertyPath(propertyTestPlugin, propertyTestApp)
+	Expect(makePluginAppPropertyPath(propertyTestPlugin, propertyTestApp)).To(Succeed())
+	t.Setenv(LockedDirectoriesEnvVar, "/some/other/dir\n"+dir+"/")
+
+	done := make(chan error, 1)
+	go func() {
+		done <- LockDirectory(dir, func() error {
+			return PropertyWrite(propertyTestPlugin, propertyTestApp, "nested", "value")
+		})
+	}()
+
+	select {
+	case err := <-done:
+		Expect(err).NotTo(HaveOccurred())
+	case <-time.After(5 * time.Second):
+		t.Fatal("nested lock on a directory listed in DOKKU_LOCKED_DIRECTORIES blocked")
+	}
+	Expect(PropertyGet(propertyTestPlugin, propertyTestApp, "nested")).To(Equal("value"))
+}
+
+func TestPropertyWritesLeaveNoTemporaryFiles(t *testing.T) {
+	setupPropertyMapTest(t)
+
+	Expect(PropertyWrite(propertyTestPlugin, propertyTestApp, "plain", "value")).To(Succeed())
+	Expect(PropertyListWrite(propertyTestPlugin, propertyTestApp, "list", []string{"a"})).To(Succeed())
+	Expect(PropertyMapSet(propertyTestPlugin, propertyTestApp, propertyTestName, "key", "value")).To(Succeed())
+
+	entries, err := os.ReadDir(getPluginAppPropertyPath(propertyTestPlugin, propertyTestApp))
+	Expect(err).NotTo(HaveOccurred())
+	names := []string{}
+	for _, entry := range entries {
+		names = append(names, entry.Name())
+	}
+	Expect(names).To(ConsistOf("plain", "list", propertyTestName))
+}
+
+func TestPropertyGetAllIgnoresHiddenFiles(t *testing.T) {
+	setupPropertyMapTest(t)
+
+	Expect(PropertyWrite(propertyTestPlugin, propertyTestApp, "plain", "value")).To(Succeed())
+	stray := filepath.Join(getPluginAppPropertyPath(propertyTestPlugin, propertyTestApp), ".plain.tmp-123")
+	Expect(os.WriteFile(stray, []byte("partial"), 0600)).To(Succeed())
+
+	all, err := PropertyGetAll(propertyTestPlugin, propertyTestApp)
+	Expect(err).NotTo(HaveOccurred())
+	Expect(all).To(Equal(map[string]string{"plain": "value"}))
+
+	byPrefix, err := PropertyGetAllByPrefix(propertyTestPlugin, propertyTestApp, "")
+	Expect(err).NotTo(HaveOccurred())
+	Expect(byPrefix).To(Equal(map[string]string{"plain": "value"}))
+}
+
+// runConcurrently calls fn with 0 through n-1 from n goroutines started
+// together, returning the joined errors of every call.
+func runConcurrently(n int, fn func(i int) error) error {
+	var wg sync.WaitGroup
+	start := make(chan struct{})
+	errs := make([]error, n)
+	for i := 0; i < n; i++ {
+		wg.Add(1)
+		go func(i int) {
+			defer wg.Done()
+			<-start
+			errs[i] = fn(i)
+		}(i)
+	}
+	close(start)
+	wg.Wait()
+	return errors.Join(errs...)
+}

plugins/common/property-functions

@@ -116,3 +116,43 @@ fn-plugin-property-setup-app() {
   declare PLUGIN="$1" APP="$2"
   "$PLUGIN_CORE_AVAILABLE_PATH/common/prop" "setup-app" "$PLUGIN" "$APP"
 }
+
+fn-plugin-property-lock() {
+  declare desc="runs a callback while holding the exclusive property lock for an app"
+  declare PLUGIN="$1" APP="$2" CALLBACK="$3"
+  shift 3
+  local lock_dir="${DOKKU_LIB_ROOT}/config/${PLUGIN}/${APP}"
+  local lock_fd previous_locked_directories="$DOKKU_LOCKED_DIRECTORIES"
+
+  if fn-plugin-property-lock-held "$lock_dir"; then
+    "$CALLBACK" "$@"
+    return
+  fi
+
+  fn-plugin-property-setup-app "$PLUGIN" "$APP"
+  exec {lock_fd}<"$lock_dir"
+  flock "$lock_fd"
+
+  export DOKKU_LOCKED_DIRECTORIES="${previous_locked_directories:+$previous_locked_directories$'\n'}$lock_dir"
+  "$CALLBACK" "$@"
+  local status=$?
+
+  if [[ -n "$previous_locked_directories" ]]; then
+    export DOKKU_LOCKED_DIRECTORIES="$previous_locked_directories"
+  else
+    unset DOKKU_LOCKED_DIRECTORIES
+  fi
+  exec {lock_fd}<&-
+  return "$status"
+}
+
+fn-plugin-property-lock-held() {
+  declare desc="returns whether a property lock directory is already held by a parent process"
+  declare LOCK_DIR="$1"
+  local held
+
+  while IFS= read -r held; do
+    [[ -n "$held" ]] && [[ "$held" == "$LOCK_DIR" ]] && return 0
+  done <<<"$DOKKU_LOCKED_DIRECTORIES"
+  return 1
+}

plugins/config/concurrency_test.go

@@ -0,0 +1,62 @@
+package config
+
+import (
+	"errors"
+	"fmt"
+	"sync"
+	"testing"
+
+	. "github.com/onsi/gomega"
+)
+
+func TestConfigSetManyConcurrent(t *testing.T) {
+	RegisterTestingT(t)
+	setupTestApp(t)
+
+	const count = 20
+	var wg sync.WaitGroup
+	errs := make([]error, count)
+	for i := 0; i < count; i++ {
+		wg.Add(1)
+		go func(i int) {
+			defer wg.Done()
+			errs[i] = SetMany(testAppName, map[string]string{fmt.Sprintf("KEY%d", i): "value"}, false, false)
+		}(i)
+	}
+	wg.Wait()
+	Expect(errors.Join(errs...)).NotTo(HaveOccurred())
+
+	for i := 0; i < count; i++ {
+		expectValue(testAppName, fmt.Sprintf("KEY%d", i), "value")
+	}
+	expectValue(testAppName, "testKey", "TESTING")
+}
+
+func TestConfigUnsetManyConcurrent(t *testing.T) {
+	RegisterTestingT(t)
+	setupTestApp(t)
+
+	const count = 20
+	entries := map[string]string{}
+	for i := 0; i < count; i++ {
+		entries[fmt.Sprintf("KEY%d", i)] = "value"
+	}
+	Expect(SetMany(testAppName, entries, false, false)).To(Succeed())
+
+	var wg sync.WaitGroup
+	errs := make([]error, count)
+	for i := 0; i < count; i++ {
+		wg.Add(1)
+		go func(i int) {
+			defer wg.Done()
+			errs[i] = UnsetMany(testAppName, []string{fmt.Sprintf("KEY%d", i)}, false)
+		}(i)
+	}
+	wg.Wait()
+	Expect(errors.Join(errs...)).NotTo(HaveOccurred())
+
+	for i := 0; i < count; i++ {
+		expectNoValue(testAppName, fmt.Sprintf("KEY%d", i))
+	}
+	expectValue(testAppName, "testKey", "TESTING")
+}

plugins/config/config.go

@@ -3,6 +3,7 @@ package config
 import (
 	"fmt"
 	"os"
+	"path/filepath"
 	"regexp"
 
 	"github.com/dokku/dokku/plugins/common"
@@ -32,10 +33,6 @@ func GetWithDefault(appName string, key string, defaultValue string) (value stri
 // SetMany variables in the environment. If appName is empty the global config is used. If restart is true the app is restarted.
 func SetMany(appName string, entries map[string]string, replace bool, restart bool) (err error) {
 	global := appName == "" || appName == "--global"
-	env, err := loadAppOrGlobalEnv(appName)
-	if err != nil {
-		return
-	}
 	keys := make([]string, 0, len(entries))
 	for k := range entries {
 		if err = validateKey(k); err != nil {
@@ -43,21 +40,28 @@ func SetMany(appName string, entries map[string]string, replace bool, restart bo
 		}
 	}
 
-	if replace {
-		env.Clear()
-	}
-	for k, v := range entries {
-		env.Set(k, v)
-		keys = append(keys, k)
-	}
-	if len(entries) != 0 {
+	changed, err := updateAppOrGlobalEnv(appName, func(env *Env) bool {
+		if replace {
+			env.Clear()
+		}
+		for k, v := range entries {
+			env.Set(k, v)
+			keys = append(keys, k)
+		}
+		if len(entries) == 0 {
+			return false
+		}
+
 		common.LogInfo1Quiet("Setting config vars")
 		if os.Getenv("DOKKU_QUIET_OUTPUT") == "" {
 			fmt.Println(prettyPrintEnvEntries("       ", entries))
 		}
-		if err := env.Write(); err != nil {
-			return fmt.Errorf("Unable to write config vars: %s", err.Error())
-		}
+		return true
+	})
+	if err != nil {
+		return err
+	}
+	if changed {
 		triggerUpdate(appName, "set", keys)
 	}
 	if !global && restart && shouldRestart(appName) {
@@ -69,29 +73,28 @@ func SetMany(appName string, entries map[string]string, replace bool, restart bo
 // UnsetMany a value in a config. If appName is empty the global config is used. If restart is true the app is restarted.
 func UnsetMany(appName string, keys []string, restart bool) (err error) {
 	global := appName == "" || appName == "--global"
-	env, err := loadAppOrGlobalEnv(appName)
-	if err != nil {
-		return
-	}
-	var changed = false
 	for _, k := range keys {
 		if err = validateKey(k); err != nil {
 			return
 		}
 	}
-	for _, k := range keys {
-		if _, hasKey := env.Map()[k]; hasKey {
-			common.LogInfo1Quiet(fmt.Sprintf("Unsetting %s", k))
-			env.Unset(k)
-			changed = true
-		} else {
-			common.LogInfo1Quiet(fmt.Sprintf("Skipping %s, it is not set in the environment", k))
+	changed, err := updateAppOrGlobalEnv(appName, func(env *Env) bool {
+		changed := false
+		for _, k := range keys {
+			if _, hasKey := env.Map()[k]; hasKey {
+				common.LogInfo1Quiet(fmt.Sprintf("Unsetting %s", k))
+				env.Unset(k)
+				changed = true
+			} else {
+				common.LogInfo1Quiet(fmt.Sprintf("Skipping %s, it is not set in the environment", k))
+			}
 		}
+		return changed
+	})
+	if err != nil {
+		return err
 	}
 	if changed {
-		if err := env.Write(); err != nil {
-			return fmt.Errorf("Unable to write config vars: %s", err.Error())
-		}
 		triggerUpdate(appName, "unset", keys)
 	}
 	if !global && restart && shouldRestart(appName) {
@@ -103,20 +106,19 @@ func UnsetMany(appName string, keys []string, restart bool) (err error) {
 // UnsetAll removes all config keys
 func UnsetAll(appName string, restart bool) (err error) {
 	global := appName == "" || appName == "--global"
-	env, err := loadAppOrGlobalEnv(appName)
+	changed, err := updateAppOrGlobalEnv(appName, func(env *Env) bool {
+		changed := false
+		for k := range env.Map() {
+			common.LogInfo1Quiet(fmt.Sprintf("Unsetting %s", k))
+			env.Unset(k)
+			changed = true
+		}
+		return changed
+	})
 	if err != nil {
-		return
-	}
-	var changed = false
-	for k := range env.Map() {
-		common.LogInfo1Quiet(fmt.Sprintf("Unsetting %s", k))
-		env.Unset(k)
-		changed = true
+		return err
 	}
 	if changed {
-		if err := env.Write(); err != nil {
-			return fmt.Errorf("Unable to write config vars: %s", err.Error())
-		}
 		triggerUpdate(appName, "clear", []string{})
 	}
 	if !global && restart && shouldRestart(appName) {
@@ -125,6 +127,48 @@ func UnsetAll(appName string, restart bool) (err error) {
 	return
 }
 
+// updateAppOrGlobalEnv loads the app or global environment, passes it to
+// update and writes it back when update reports a change, returning whether it
+// did. The directory holding the environment file stays locked from the load
+// to the write, so concurrent config changes cannot drop each other's keys.
+// Triggers reacting to the change must run after this returns, as a trigger
+// that changes the same environment would otherwise wait on the lock forever.
+func updateAppOrGlobalEnv(appName string, update func(env *Env) bool) (bool, error) {
+	filename := getGlobalFile()
+	if appName != "" && appName != "--global" {
+		var err error
+		if filename, err = getAppFile(appName); err != nil {
+			return false, err
+		}
+	}
+
+	changed := false
+	apply := func() error {
+		env, err := loadAppOrGlobalEnv(appName)
+		if err != nil {
+			return err
+		}
+
+		if !update(env) {
+			return nil
+		}
+
+		if err := env.Write(); err != nil {
+			return fmt.Errorf("Unable to write config vars: %s", err.Error())
+		}
+		changed = true
+		return nil
+	}
+
+	dir := filepath.Dir(filename)
+	if !common.DirectoryExists(dir) {
+		return changed, apply()
+	}
+
+	err := common.LockDirectory(dir, apply)
+	return changed, err
+}
+
 func shouldRestart(appName string) bool {
 	results, _ := common.CallPlugnTrigger(common.PlugnTriggerInput{
 		Trigger: "ps-get-property",

plugins/config/migrate.go

@@ -54,12 +54,14 @@ func migrateGlobalEnv() error {
 	}
 
 	oldGlobalEnvFile := filepath.Join(common.MustGetEnv("DOKKU_ROOT"), "ENV")
-	globalEnv, err := LoadGlobalEnv()
-	if err != nil {
-		return fmt.Errorf("Unable to load global environment: %s", err.Error())
-	}
+	return common.LockDirectory(filepath.Dir(getGlobalFile()), func() error {
+		globalEnv, err := LoadGlobalEnv()
+		if err != nil {
+			return fmt.Errorf("Unable to load global environment: %s", err.Error())
+		}
 
-	return drainLegacyEnvFile("--global", oldGlobalEnvFile, globalEnv)
+		return drainLegacyEnvFile("--global", oldGlobalEnvFile, globalEnv)
+	})
 }
 
 // migrateAppEnv drains $DOKKU_ROOT/<app>/ENV into the app's config property path.
@@ -72,13 +74,20 @@ func migrateAppEnv(appName string) error {
 		return fmt.Errorf("Unable to setup app config directory: %s", err.Error())
 	}
 
-	env, err := LoadAppEnv(appName)
+	appFile, err := getAppFile(appName)
 	if err != nil {
-		return fmt.Errorf("Unable to load app environment: %s", err.Error())
+		return err
 	}
 
 	oldEnvFile := filepath.Join(common.AppRoot(appName), "ENV")
-	return drainLegacyEnvFile(appName, oldEnvFile, env)
+	return common.LockDirectory(filepath.Dir(appFile), func() error {
+		env, err := LoadAppEnv(appName)
+		if err != nil {
+			return fmt.Errorf("Unable to load app environment: %s", err.Error())
+		}
+
+		return drainLegacyEnvFile(appName, oldEnvFile, env)
+	})
 }
 
 // drainLegacyEnvFile merges the legacy ENV file at oldEnvFile into env, records

plugins/docker-options/concurrency_test.go

@@ -0,0 +1,72 @@
+package dockeroptions
+
+import (
+	"errors"
+	"fmt"
+	"sync"
+	"testing"
+)
+
+func TestAddDockerOptionToPhasesConcurrent(t *testing.T) {
+	const appName = "race-test"
+	setupReportEnv(t, appName)
+
+	const count = 20
+	var wg sync.WaitGroup
+	errs := make([]error, count)
+	for i := 0; i < count; i++ {
+		wg.Add(1)
+		go func(i int) {
+			defer wg.Done()
+			errs[i] = AddDockerOptionToPhases(appName, []string{"deploy", "run"}, fmt.Sprintf("--label=race%d", i))
+		}(i)
+	}
+	wg.Wait()
+	if err := errors.Join(errs...); err != nil {
+		t.Fatalf("AddDockerOptionToPhases: %v", err)
+	}
+
+	for _, phase := range []string{"deploy", "run"} {
+		options, err := GetDockerOptionsForPhase(appName, phase)
+		if err != nil {
+			t.Fatalf("GetDockerOptionsForPhase(%s): %v", phase, err)
+		}
+		if len(options) != count {
+			t.Fatalf("%s phase has %d options, want %d: %v", phase, len(options), count, options)
+		}
+	}
+}
+
+func TestRemoveDockerOptionFromPhasesConcurrent(t *testing.T) {
+	const appName = "race-test"
+	setupReportEnv(t, appName)
+
+	const count = 20
+	for i := 0; i < count; i++ {
+		if err := AddDockerOptionToPhases(appName, []string{"deploy"}, fmt.Sprintf("--label=race%d", i)); err != nil {
+			t.Fatalf("AddDockerOptionToPhases: %v", err)
+		}
+	}
+
+	var wg sync.WaitGroup
+	errs := make([]error, count)
+	for i := 0; i < count; i++ {
+		wg.Add(1)
+		go func(i int) {
+			defer wg.Done()
+			errs[i] = RemoveDockerOptionFromPhases(appName, []string{"deploy"}, fmt.Sprintf("--label=race%d", i))
+		}(i)
+	}
+	wg.Wait()
+	if err := errors.Join(errs...); err != nil {
+		t.Fatalf("RemoveDockerOptionFromPhases: %v", err)
+	}
+
+	options, err := GetDockerOptionsForPhase(appName, "deploy")
+	if err != nil {
+		t.Fatalf("GetDockerOptionsForPhase: %v", err)
+	}
+	if len(options) != 0 {
+		t.Fatalf("deploy phase still has options: %v", options)
+	}
+}

plugins/docker-options/dockeroptions.go

@@ -198,32 +198,17 @@ func SetDockerOptionForPhases(appName string, phases []string, name string, valu
 // SetDockerOptionForProcessPhases sets a `--name=value` option for the specified
 // process types and phases, replacing any existing entry with the same name.
 func SetDockerOptionForProcessPhases(appName string, processTypes []string, phases []string, name string, value string) error {
-	if len(processTypes) == 0 {
-		processTypes = []string{DefaultProcessType}
-	}
-	for _, processType := range processTypes {
-		for _, phase := range phases {
-			options, err := GetDockerOptionsForProcessPhase(appName, processType, phase)
-			if err != nil {
-				return err
-			}
-
-			newOptions := []string{}
-			for _, option := range options {
-				if strings.HasPrefix(option, fmt.Sprintf("--%s=", name)) {
-					continue
-				}
-				newOptions = append(newOptions, option)
-			}
-
-			newOptions = append(newOptions, fmt.Sprintf("--%s=%s", name, value))
-			sort.Strings(newOptions)
-			if err := writeDockerOptionsForProcessPhase(appName, processType, phase, newOptions); err != nil {
-				return err
+	return updateDockerOptionsForProcessPhases(appName, processTypes, phases, func(options []string) []string {
+		newOptions := []string{}
+		for _, option := range options {
+			if strings.HasPrefix(option, fmt.Sprintf("--%s=", name)) {
+				continue
 			}
+			newOptions = append(newOptions, option)
 		}
-	}
-	return nil
+
+		return append(newOptions, fmt.Sprintf("--%s=%s", name, value))
+	})
 }
 
 // AddDockerOptionToPhases adds an option to the default scope for the specified phases.
@@ -233,24 +218,9 @@ func AddDockerOptionToPhases(appName string, phases []string, option string) err
 
 // AddDockerOptionToProcessPhases adds an option to the specified process types and phases.
 func AddDockerOptionToProcessPhases(appName string, processTypes []string, phases []string, option string) error {
-	if len(processTypes) == 0 {
-		processTypes = []string{DefaultProcessType}
-	}
-	for _, processType := range processTypes {
-		for _, phase := range phases {
-			options, err := GetDockerOptionsForProcessPhase(appName, processType, phase)
-			if err != nil {
-				return err
-			}
-
-			options = append(options, option)
-			sort.Strings(options)
-			if err := writeDockerOptionsForProcessPhase(appName, processType, phase, options); err != nil {
-				return err
-			}
-		}
-	}
-	return nil
+	return updateDockerOptionsForProcessPhases(appName, processTypes, phases, func(options []string) []string {
+		return append(options, option)
+	})
 }
 
 // GetDockerOptionsForPhase returns the docker options stored under the default
@@ -290,30 +260,15 @@ func RemoveDockerOptionFromPhases(appName string, phases []string, option string
 // non-canonical form still matches the canonically re-serialized string the
 // CLI hands down.
 func RemoveDockerOptionFromProcessPhases(appName string, processTypes []string, phases []string, option string) error {
-	if len(processTypes) == 0 {
-		processTypes = []string{DefaultProcessType}
-	}
-	for _, processType := range processTypes {
-		for _, phase := range phases {
-			options, err := GetDockerOptionsForProcessPhase(appName, processType, phase)
-			if err != nil {
-				return err
-			}
-
-			newOptions := []string{}
-			for _, opt := range options {
-				if !optionsEqual(opt, option) {
-					newOptions = append(newOptions, opt)
-				}
-			}
-
-			sort.Strings(newOptions)
-			if err := writeDockerOptionsForProcessPhase(appName, processType, phase, newOptions); err != nil {
-				return err
+	return updateDockerOptionsForProcessPhases(appName, processTypes, phases, func(options []string) []string {
+		newOptions := []string{}
+		for _, opt := range options {
+			if !optionsEqual(opt, option) {
+				newOptions = append(newOptions, opt)
 			}
 		}
-	}
-	return nil
+		return newOptions
+	})
 }
 
 // GetSpecifiedDockerOptionsForPhase returns the docker options for the specified
@@ -410,6 +365,31 @@ func splitPropertyKey(key string) (processType, phase string, ok bool) {
 	return processType, phase, true
 }
 
-func writeDockerOptionsForProcessPhase(appName, processType, phase string, options []string) error {
-	return common.PropertyListWrite("docker-options", appName, propertyKey(processType, phase), options)
+// updateDockerOptionsForProcessPhases rewrites the stored options of every
+// process type and phase combination with the sorted result of update, holding
+// the app's docker-options property lock across every read and write so
+// concurrent updates to the same app cannot drop each other's options. An
+// empty processTypes list is treated as the default scope.
+func updateDockerOptionsForProcessPhases(appName string, processTypes []string, phases []string, update func(options []string) []string) error {
+	if len(processTypes) == 0 {
+		processTypes = []string{DefaultProcessType}
+	}
+
+	return common.PropertyLock("docker-options", appName, func(p *common.LockedProperties) error {
+		for _, processType := range processTypes {
+			for _, phase := range phases {
+				options, err := GetDockerOptionsForProcessPhase(appName, processType, phase)
+				if err != nil {
+					return err
+				}
+
+				options = update(options)
+				sort.Strings(options)
+				if err := p.ListWrite(propertyKey(processType, phase), options); err != nil {
+					return err
+				}
+			}
+		}
+		return nil
+	})
 }

plugins/docker-options/functions.go

@@ -390,36 +390,42 @@ func migrateTraefikLabelBackticks() error {
 	}
 
 	for _, appName := range apps {
-		properties, err := common.PropertyGetAll("docker-options", appName)
-		if err != nil {
-			return err
-		}
-
-		for key := range properties {
-			processType, phase, ok := splitPropertyKey(key)
-			if !ok {
-				continue
-			}
-
-			options, err := GetDockerOptionsForProcessPhase(appName, processType, phase)
+		err := common.PropertyLock("docker-options", appName, func(p *common.LockedProperties) error {
+			properties, err := common.PropertyGetAll("docker-options", appName)
 			if err != nil {
 				return err
 			}
 
-			changed := false
-			for i, option := range options {
-				if fixed, ok := repairTraefikLabelBackticks(option); ok {
-					options[i] = fixed
-					changed = true
+			for key := range properties {
+				processType, phase, ok := splitPropertyKey(key)
+				if !ok {
+					continue
 				}
-			}
 
-			if changed {
-				if err := writeDockerOptionsForProcessPhase(appName, processType, phase, options); err != nil {
+				options, err := GetDockerOptionsForProcessPhase(appName, processType, phase)
+				if err != nil {
 					return err
 				}
-				common.LogInfo1(fmt.Sprintf("Repaired docker-options label backticks for %s (%s %s)", appName, processType, phase))
+
+				changed := false
+				for i, option := range options {
+					if fixed, ok := repairTraefikLabelBackticks(option); ok {
+						options[i] = fixed
+						changed = true
+					}
+				}
+
+				if changed {
+					if err := p.ListWrite(key, options); err != nil {
+						return err
+					}
+					common.LogInfo1(fmt.Sprintf("Repaired docker-options label backticks for %s (%s %s)", appName, processType, phase))
+				}
 			}
+			return nil
+		})
+		if err != nil {
+			return err
 		}
 	}
 
@@ -494,31 +500,37 @@ func migrateNonCanonicalOptions() error {
 	}
 
 	for _, appName := range apps {
-		properties, err := common.PropertyGetAll("docker-options", appName)
-		if err != nil {
-			return err
-		}
-
-		for key := range properties {
-			processType, phase, ok := splitPropertyKey(key)
-			if !ok {
-				continue
-			}
-
-			options, err := GetDockerOptionsForProcessPhase(appName, processType, phase)
+		err := common.PropertyLock("docker-options", appName, func(p *common.LockedProperties) error {
+			properties, err := common.PropertyGetAll("docker-options", appName)
 			if err != nil {
 				return err
 			}
 
-			canonical := canonicalizeOptionLines(options)
-			if slices.Equal(canonical, options) {
-				continue
-			}
+			for key := range properties {
+				processType, phase, ok := splitPropertyKey(key)
+				if !ok {
+					continue
+				}
 
-			if err := writeDockerOptionsForProcessPhase(appName, processType, phase, canonical); err != nil {
-				return err
+				options, err := GetDockerOptionsForProcessPhase(appName, processType, phase)
+				if err != nil {
+					return err
+				}
+
+				canonical := canonicalizeOptionLines(options)
+				if slices.Equal(canonical, options) {
+					continue
+				}
+
+				if err := p.ListWrite(key, canonical); err != nil {
+					return err
+				}
+				common.LogInfo1(fmt.Sprintf("Rewrote docker-options into canonical form for %s (%s %s)", appName, processType, phase))
 			}
-			common.LogInfo1(fmt.Sprintf("Rewrote docker-options into canonical form for %s (%s %s)", appName, processType, phase))
+			return nil
+		})
+		if err != nil {
+			return err
 		}
 	}
 

plugins/git/functions

@@ -175,11 +175,22 @@ fn-git-build-with-lock-held() {
 
   (git_build "$APP" "$REV") || exit_code="$?"
   if [[ "$exit_code" -ne 0 ]] && [[ "$has_source_image" == "true" ]]; then
-    fn-plugin-property-write "git" "$APP" "source-image" "$previous_source_image"
+    fn-plugin-property-lock "git" "$APP" fn-git-restore-source-image "$APP" "$source_image" "$previous_source_image"
   fi
   return "$exit_code"
 }
 
+fn-git-restore-source-image() {
+  declare desc="restores the previous source-image unless it was changed since the failed build set it, expecting the caller to hold the git property lock"
+  declare APP="$1" BUILD_SOURCE_IMAGE="$2" PREVIOUS_SOURCE_IMAGE="$3"
+
+  if [[ "$(fn-git-source-image "$APP")" != "$BUILD_SOURCE_IMAGE" ]]; then
+    return
+  fi
+
+  fn-plugin-property-write "git" "$APP" "source-image" "$PREVIOUS_SOURCE_IMAGE"
+}
+
 cmd-git-upload-pack() {
   declare desc="executes git-upload-pack"
   declare cmd="git-upload-pack"

plugins/nginx-vhosts/functions

@@ -428,6 +428,20 @@ fn-nginx-vhosts-pre-validate-custom-template() {
   fi
 }
 
+fn-nginx-vhosts-install-config() {
+  declare desc="atomically replaces the nginx.conf for an app with a rendered config"
+  declare APP="$1" RENDERED_CONF="$2"
+  local STAGED_CONF
+
+  STAGED_CONF="$(mktemp "$DOKKU_ROOT/$APP/.nginx.conf.XXXXXX")"
+  if ! cat "$RENDERED_CONF" >"$STAGED_CONF"; then
+    rm -f "$STAGED_CONF"
+    return 1
+  fi
+  mv -f "$STAGED_CONF" "$DOKKU_ROOT/$APP/nginx.conf"
+  rm -f "$RENDERED_CONF"
+}
+
 nginx_build_config() {
   declare desc="build nginx config to proxy app containers using sigil"
   declare APP="$1" DOKKU_APP_LISTEN_PORT="$2" DOKKU_APP_LISTEN_IP="$3"
@@ -545,7 +559,7 @@ nginx_build_config() {
 
       dokku_log_info1 "Creating $SCHEME nginx.conf"
       mkdir -p "$DOKKU_ROOT/$APP/nginx.conf.d"
-      mv "$NGINX_CONF" "$DOKKU_ROOT/$APP/nginx.conf"
+      fn-nginx-vhosts-install-config "$APP" "$NGINX_CONF"
 
       if [[ "$IS_DEPLOYED_WITH_LISTENERS" == "true" ]]; then
         fn-nginx-vhosts-manage-hsts "$APP" "$SSL_INUSE"

plugins/ports/functions.go

@@ -15,10 +15,18 @@ import (
 
 // addPortMaps adds port mappings to an app
 func addPortMaps(appName string, portMaps []PortMap) error {
+	return common.PropertyLock("ports", appName, func(p *common.LockedProperties) error {
+		return addPortMapsWithLock(p, appName, portMaps)
+	})
+}
+
+// addPortMapsWithLock adds port mappings to an app. The caller must hold the
+// app's ports property lock.
+func addPortMapsWithLock(p *common.LockedProperties, appName string, portMaps []PortMap) error {
 	allPortMaps := getPortMaps(appName)
 	allPortMaps = append(allPortMaps, portMaps...)
 
-	return setPortMaps(appName, allPortMaps)
+	return writePortMaps(p, allPortMaps)
 }
 
 // clearPorts clears all port mappings for an app
@@ -365,6 +373,14 @@ func parsePortMapString(stringPortMap string) ([]PortMap, error) {
 
 // removePortMaps removes specific port mappings from an app
 func removePortMaps(appName string, portMaps []PortMap) error {
+	return common.PropertyLock("ports", appName, func(p *common.LockedProperties) error {
+		return removePortMapsWithLock(p, appName, portMaps)
+	})
+}
+
+// removePortMapsWithLock removes specific port mappings from an app. The caller
+// must hold the app's ports property lock.
+func removePortMapsWithLock(p *common.LockedProperties, appName string, portMaps []PortMap) error {
 	toRemove := map[string]bool{}
 	toRemoveByPort := map[int]bool{}
 
@@ -390,10 +406,10 @@ func removePortMaps(appName string, portMaps []PortMap) error {
 	}
 
 	if len(toSet) == 0 {
-		return common.PropertyDelete("ports", appName, "map")
+		return p.Delete("map")
 	}
 
-	return setPortMaps(appName, toSet)
+	return writePortMaps(p, toSet)
 }
 
 // reusesSchemeHostPort returns true if the port maps reuse the same scheme:host-port
@@ -413,6 +429,14 @@ func reusesSchemeHostPort(portMaps []PortMap) error {
 
 // setPortMaps sets the port maps for an app
 func setPortMaps(appName string, portMaps []PortMap) error {
+	return common.PropertyLock("ports", appName, func(p *common.LockedProperties) error {
+		return writePortMaps(p, portMaps)
+	})
+}
+
+// writePortMaps writes the port maps for an app. The caller must hold the
+// app's ports property lock.
+func writePortMaps(p *common.LockedProperties, portMaps []PortMap) error {
 	if err := reusesSchemeHostPort(portMaps); err != nil {
 		return fmt.Errorf("Error validating port mappings: %s", err)
 	}
@@ -427,7 +451,7 @@ func setPortMaps(appName string, portMaps []PortMap) error {
 	}
 
 	sort.Strings(value)
-	return common.PropertyListWrite("ports", appName, "map", value)
+	return p.ListWrite("map", value)
 }
 
 // setProxyPort sets the proxy port for an app

plugins/ports/triggers.go

@@ -160,11 +160,21 @@ func TriggerPostCertsRemove(appName string) error {
 		return err
 	}
 
-	return removePortMaps(appName, filterAppPortMaps(appName, "https", 443))
+	return common.PropertyLock("ports", appName, func(p *common.LockedProperties) error {
+		return removePortMapsWithLock(p, appName, filterAppPortMaps(appName, "https", 443))
+	})
 }
 
 // TriggerPostCertsUpdate sets port properties after SSL cert is added
 func TriggerPostCertsUpdate(appName string) error {
+	return common.PropertyLock("ports", appName, func(p *common.LockedProperties) error {
+		return postCertsUpdateWithLock(p, appName)
+	})
+}
+
+// postCertsUpdateWithLock adds https:443 port maps mirroring the app's http:80
+// port maps. The caller must hold the app's ports property lock.
+func postCertsUpdateWithLock(p *common.LockedProperties, appName string) error {
 	port := common.PropertyGet("proxy", appName, "proxy-port")
 	sslPort := common.PropertyGet("proxy", appName, "proxy-ssl-port")
 	portMaps := getPortMaps(appName)
@@ -201,7 +211,7 @@ func TriggerPostCertsUpdate(appName string) error {
 			})
 		}
 
-		if err := addPortMaps(appName, toAdd); err != nil {
+		if err := addPortMapsWithLock(p, appName, toAdd); err != nil {
 			return err
 		}
 	}

plugins/proxy/command-functions

@@ -1,5 +1,6 @@
 #!/usr/bin/env bash
 source "$PLUGIN_CORE_AVAILABLE_PATH/common/functions"
+source "$PLUGIN_CORE_AVAILABLE_PATH/common/property-functions"
 source "$PLUGIN_AVAILABLE_PATH/proxy/functions"
 set -eo pipefail
 [[ $DOKKU_TRACE ]] && set -x
@@ -32,12 +33,7 @@ cmd-proxy-labels-add() {
   [[ -z "$PASSED_LABEL_VALUE" ]] && dokku_log_fail "Please specify a value for the container label"
   verify_app_name "$APP"
 
-  local proxy_labels_file_path="$(fn-proxy-get-labels-file-path "$PROXY" "$APP")"
-  touch "$proxy_labels_file_path"
-  cmd-proxy-labels-remove "$PROXY" "$APP" "$PASSED_LABEL_NAME" >/dev/null
-  echo "${PASSED_LABEL_NAME}=${PASSED_LABEL_VALUE}" >>"$proxy_labels_file_path"
-  local all_proxy_labels="$(<"$proxy_labels_file_path")"
-  echo -e "${all_proxy_labels}" | sed '/^$/d' | sort -u >"$proxy_labels_file_path"
+  fn-plugin-property-lock "$PROXY" "$APP" fn-proxy-labels-add "$PROXY" "$APP" "$PASSED_LABEL_NAME" "$PASSED_LABEL_VALUE"
 }
 
 cmd-proxy-labels-remove() {
@@ -55,8 +51,5 @@ cmd-proxy-labels-remove() {
     return
   fi
 
-  local all_proxy_labels="$(<"$proxy_labels_file_path")"
-  local escaped_label_name="$(fn-proxy-escape-extended-sed "$PASSED_LABEL_NAME")"
-  local all_proxy_labels="$(echo -e "${all_proxy_labels}" | sed -E "s/^${escaped_label_name}=.*\$//")"
-  echo -e "${all_proxy_labels}" | sed '/^$/d' | sort -u >"$proxy_labels_file_path"
+  fn-plugin-property-lock "$PROXY" "$APP" fn-proxy-labels-remove "$PROXY" "$APP" "$PASSED_LABEL_NAME"
 }

plugins/proxy/functions

@@ -56,3 +56,57 @@ fn-proxy-show-label() {
 
   echo "$proxy_label_value"
 }
+
+fn-proxy-labels-add() {
+  declare desc="adds a container label for the specified proxy, expecting the caller to hold the proxy property lock"
+  declare PROXY="$1" APP="$2" LABEL_NAME="$3" LABEL_VALUE="$4"
+  local proxy_labels_file_path="$(fn-proxy-get-labels-file-path "$PROXY" "$APP")"
+  local all_proxy_labels=""
+
+  if [[ -s "$proxy_labels_file_path" ]]; then
+    all_proxy_labels="$(fn-proxy-labels-without "$proxy_labels_file_path" "$LABEL_NAME")"
+  fi
+
+  all_proxy_labels="${all_proxy_labels}"$'\n'"${LABEL_NAME}=${LABEL_VALUE}"
+  echo -e "${all_proxy_labels}" | sed '/^$/d' | sort -u | fn-proxy-write-labels-file "$proxy_labels_file_path"
+}
+
+fn-proxy-labels-remove() {
+  declare desc="removes a container label for the specified proxy, expecting the caller to hold the proxy property lock"
+  declare PROXY="$1" APP="$2" LABEL_NAME="$3"
+  local proxy_labels_file_path="$(fn-proxy-get-labels-file-path "$PROXY" "$APP")"
+
+  if [[ ! -s "$proxy_labels_file_path" ]]; then
+    return
+  fi
+
+  fn-proxy-labels-without "$proxy_labels_file_path" "$LABEL_NAME" | fn-proxy-write-labels-file "$proxy_labels_file_path"
+}
+
+fn-proxy-labels-without() {
+  declare desc="prints the sorted labels in a proxy labels file without the named label"
+  declare LABELS_FILE="$1" LABEL_NAME="$2"
+  local all_proxy_labels="$(<"$LABELS_FILE")"
+  local escaped_label_name="$(fn-proxy-escape-extended-sed "$LABEL_NAME")"
+
+  echo -e "${all_proxy_labels}" | sed -E "s/^${escaped_label_name}=.*\$//" | sed '/^$/d' | sort -u
+}
+
+fn-proxy-write-labels-file() {
+  declare desc="atomically replaces a proxy labels file with the contents of stdin"
+  declare LABELS_FILE="$1"
+  local tmp_file
+
+  tmp_file="$(mktemp "$(dirname "$LABELS_FILE")/.labels.XXXXXX")"
+  if ! cat >"$tmp_file"; then
+    rm -f "$tmp_file"
+    return 1
+  fi
+
+  if [[ -f "$LABELS_FILE" ]]; then
+    chmod --reference="$LABELS_FILE" "$tmp_file"
+  else
+    chmod 0644 "$tmp_file"
+  fi
+  mv -f "$tmp_file" "$LABELS_FILE"
+}

plugins/ps/concurrency_test.go

@@ -0,0 +1,51 @@
+package ps
+
+import (
+	"errors"
+	"fmt"
+	"os/user"
+	"sync"
+	"testing"
+
+	"github.com/dokku/dokku/plugins/common"
+)
+
+func TestUpdateScaleConcurrent(t *testing.T) {
+	t.Setenv("DOKKU_LIB_ROOT", t.TempDir())
+	current, err := user.Current()
+	if err != nil {
+		t.Fatalf("user.Current: %v", err)
+	}
+	group, err := user.LookupGroupId(current.Gid)
+	if err != nil {
+		t.Fatalf("user.LookupGroupId: %v", err)
+	}
+	t.Setenv("DOKKU_SYSTEM_USER", current.Username)
+	t.Setenv("DOKKU_SYSTEM_GROUP", group.Name)
+
+	const appName = "race-test"
+	const count = 20
+	var wg sync.WaitGroup
+	errs := make([]error, count)
+	for i := 0; i < count; i++ {
+		wg.Add(1)
+		go func(i int) {
+			defer wg.Done()
+			errs[i] = updateScale(appName, false, FormationSlice{
+				{ProcessType: fmt.Sprintf("worker%d", i), Quantity: 1},
+			})
+		}(i)
+	}
+	wg.Wait()
+	if err := errors.Join(errs...); err != nil {
+		t.Fatalf("updateScale: %v", err)
+	}
+
+	tuples, err := common.PropertyListGet("ps", appName, "scale")
+	if err != nil {
+		t.Fatalf("PropertyListGet: %v", err)
+	}
+	if len(tuples) != count {
+		t.Fatalf("scale has %d process types, want %d: %v", len(tuples), count, tuples)
+	}
+}

plugins/ps/functions.go

@@ -411,7 +411,18 @@ func scaleSet(input scaleSetInput) error {
 	return nil
 }
 
+// updateScale merges formationUpdates into the stored formation, holding the
+// app's ps property lock from the read of the stored formation to the write of
+// the merged one so concurrent scale changes cannot drop each other.
 func updateScale(appName string, clearExisting bool, formationUpdates FormationSlice) error {
+	return common.PropertyLock("ps", appName, func(p *common.LockedProperties) error {
+		return updateScaleWithLock(p, appName, clearExisting, formationUpdates)
+	})
+}
+
+// updateScaleWithLock merges formationUpdates into the stored formation. The
+// caller must hold the app's ps property lock.
+func updateScaleWithLock(p *common.LockedProperties, appName string, clearExisting bool, formationUpdates FormationSlice) error {
 	formations := FormationSlice{}
 	if !clearExisting {
 		processTuples, err := common.PropertyListGet("ps", appName, "scale")
@@ -482,11 +493,11 @@ func updateScale(appName string, clearExisting bool, formationUpdates FormationS
 		values = append(values, fmt.Sprintf("%s=%d", formation.ProcessType, formation.Quantity))
 	}
 
-	if err := common.PropertyListWrite("ps", appName, "scale.old", oldValues); err != nil {
+	if err := p.ListWrite("scale.old", oldValues); err != nil {
 		return err
 	}
 
-	return common.PropertyListWrite("ps", appName, "scale", values)
+	return p.ListWrite("scale", values)
 }
 
 // validateFormations returns an error when a user specified formation cannot be scaled to.

plugins/registry/concurrency_test.go

@@ -0,0 +1,46 @@
+package registry
+
+import (
+	"errors"
+	"os/user"
+	"sort"
+	"sync"
+	"testing"
+)
+
+func TestIncrementTagVersionConcurrent(t *testing.T) {
+	t.Setenv("DOKKU_LIB_ROOT", t.TempDir())
+	current, err := user.Current()
+	if err != nil {
+		t.Fatalf("user.Current: %v", err)
+	}
+	group, err := user.LookupGroupId(current.Gid)
+	if err != nil {
+		t.Fatalf("user.LookupGroupId: %v", err)
+	}
+	t.Setenv("DOKKU_SYSTEM_USER", current.Username)
+	t.Setenv("DOKKU_SYSTEM_GROUP", group.Name)
+
+	const count = 20
+	var wg sync.WaitGroup
+	versions := make([]int, count)
+	errs := make([]error, count)
+	for i := 0; i < count; i++ {
+		wg.Add(1)
+		go func(i int) {
+			defer wg.Done()
+			versions[i], errs[i] = incrementTagVersion("race-test")
+		}(i)
+	}
+	wg.Wait()
+	if err := errors.Join(errs...); err != nil {
+		t.Fatalf("incrementTagVersion: %v", err)
+	}
+
+	sort.Ints(versions)
+	for i, version := range versions {
+		if version != i+1 {
+			t.Fatalf("versions = %v, want 1 through %d with no duplicates", versions, count)
+		}
+	}
+}

plugins/registry/functions.go

@@ -123,21 +123,28 @@ func isPushEnabled(appName string) bool {
 	return reportComputedPushOnRelease(appName) == "true"
 }
 
+// incrementTagVersion bumps the app's tag version and returns the new value,
+// holding the app's registry property lock across the read and the write so
+// concurrent releases never hand out the same version.
 func incrementTagVersion(appName string) (int, error) {
-	tag := common.PropertyGet("registry", appName, "tag-version")
-	if tag == "" {
-		tag = "0"
-	}
+	version := 0
+	err := common.PropertyLock("registry", appName, func(p *common.LockedProperties) error {
+		tag := common.PropertyGet("registry", appName, "tag-version")
+		if tag == "" {
+			tag = "0"
+		}
 
-	tag = strings.TrimSpace(tag)
-	version, err := strconv.Atoi(tag)
+		tag = strings.TrimSpace(tag)
+		current, err := strconv.Atoi(tag)
+		if err != nil {
+			return fmt.Errorf("Unable to convert existing tag version (%s) to integer: %v", tag, err)
+		}
+
+		version = current + 1
+		common.LogVerboseQuiet(fmt.Sprintf("Bumping tag to %d", version))
+		return p.Write("tag-version", strconv.Itoa(version))
+	})
 	if err != nil {
-		return 0, fmt.Errorf("Unable to convert existing tag version (%s) to integer: %v", tag, err)
-	}
-
-	version++
-	common.LogVerboseQuiet(fmt.Sprintf("Bumping tag to %d", version))
-	if err = common.PropertyWrite("registry", appName, "tag-version", strconv.Itoa(version)); err != nil {
 		return 0, err
 	}
 

plugins/registry/subcommands.go

@@ -126,7 +126,7 @@ func CommandLogin(appName string, server string, username string, password strin
 		env["DOCKER_CONFIG"] = GetAppRegistryConfigDir(appName)
 	}
 
-	result, err := common.CallExecCommand(common.ExecCommandInput{
+	result, err := execDockerConfigCommand(env["DOCKER_CONFIG"], common.ExecCommandInput{
 		Command: common.DockerBin(),
 		Args:    []string{"login", "--username", username, "--password-stdin", server},
 		Env:     env,
@@ -184,7 +184,7 @@ func CommandLogout(appName string, server string) error {
 		env["DOCKER_CONFIG"] = GetAppRegistryConfigDir(appName)
 	}
 
-	result, err := common.CallExecCommand(common.ExecCommandInput{
+	result, err := execDockerConfigCommand(env["DOCKER_CONFIG"], common.ExecCommandInput{
 		Command: common.DockerBin(),
 		Args:    []string{"logout", server},
 		Env:     env,
@@ -226,3 +226,22 @@ func CommandSet(appName string, property string, value string) error {
 	common.CommandPropertySet("registry", appName, property, value, DefaultProperties, GlobalProperties)
 	return nil
 }
+
+// execDockerConfigCommand runs a docker command that rewrites the credentials
+// file in configDir. docker merges the file it reads with the change it makes,
+// so the directory stays locked for the whole command to keep concurrent logins
+// and logouts for one app from dropping each other's credentials. An empty
+// configDir means docker's own config, which is left unlocked.
+func execDockerConfigCommand(configDir string, input common.ExecCommandInput) (common.ExecCommandResponse, error) {
+	if configDir == "" {
+		return common.CallExecCommand(input)
+	}
+
+	var result common.ExecCommandResponse
+	err := common.LockDirectory(configDir, func() error {
+		var err error
+		result, err = common.CallExecCommand(input)
+		return err
+	})
+	return result, err
+}

plugins/scheduler-k3s/subcommands.go

@@ -256,10 +256,22 @@ func CommandAutoscalingAuthSet(appName string, trigger string, metadata map[stri
 		return fmt.Errorf("Must specify at least one --metadata flag, omit --replace to remove all metadata for the trigger")
 	}
 
-	if len(metadata) == 0 || replace {
-		if err := deleteTriggerAuthProperties(appName, trigger); err != nil {
-			return err
+	err := common.PropertyLock("scheduler-k3s", appName, func(p *common.LockedProperties) error {
+		if len(metadata) == 0 || replace {
+			if err := deleteTriggerAuthProperties(p, appName, trigger); err != nil {
+				return err
+			}
 		}
+
+		for key, value := range metadata {
+			if err := p.Write(fmt.Sprintf("%s%s.%s", TriggerAuthPropertyPrefix, trigger, key), value); err != nil {
+				return fmt.Errorf("Unable to set property: %w", err)
+			}
+		}
+		return nil
+	})
+	if err != nil {
+		return err
 	}
 
 	if len(metadata) == 0 {
@@ -278,12 +290,6 @@ func CommandAutoscalingAuthSet(appName string, trigger string, metadata map[stri
 		return nil
 	}
 
-	for key, value := range metadata {
-		if err := common.PropertyWrite("scheduler-k3s", appName, fmt.Sprintf("%s%s.%s", TriggerAuthPropertyPrefix, trigger, key), value); err != nil {
-			return fmt.Errorf("Unable to set property: %w", err)
-		}
-	}
-
 	common.LogInfo1("Trigger authentication settings saved")
 	common.LogVerbose("Resources will be created or updated on next deploy")
 
@@ -297,15 +303,16 @@ func CommandAutoscalingAuthSet(appName string, trigger string, metadata map[stri
 	return nil
 }
 
-// deleteTriggerAuthProperties removes every stored metadata key for one trigger.
-func deleteTriggerAuthProperties(appName string, trigger string) error {
+// deleteTriggerAuthProperties removes every stored metadata key for one
+// trigger. The caller must hold the app's scheduler-k3s property lock.
+func deleteTriggerAuthProperties(p *common.LockedProperties, appName string, trigger string) error {
 	properties, err := common.PropertyGetAllByPrefix("scheduler-k3s", appName, fmt.Sprintf("%s%s.", TriggerAuthPropertyPrefix, trigger))
 	if err != nil {
 		return fmt.Errorf("Unable to get property list: %w", err)
 	}
 
 	for key := range properties {
-		if err := common.PropertyDelete("scheduler-k3s", appName, key); err != nil {
+		if err := p.Delete(key); err != nil {
 			return fmt.Errorf("Unable to delete property: %w", err)
 		}
 	}

plugins/scheduler-k3s/triggers.go

@@ -103,36 +103,38 @@ func TriggerInstall() error {
 // chart, and legacy values take precedence over any partial pre-existing
 // entries on re-runs.
 func migrateChartPropertiesToMapFormat() error {
-	for _, chart := range HelmCharts {
-		prefix := "chart." + chart.ReleaseName + "."
-		legacy, err := common.PropertyGetAllByPrefix("scheduler-k3s", "--global", prefix)
-		if err != nil {
-			return fmt.Errorf("Unable to read legacy chart properties for %s: %w", chart.ReleaseName, err)
-		}
-		if len(legacy) == 0 {
-			continue
-		}
+	return common.PropertyLock("scheduler-k3s", "--global", func(p *common.LockedProperties) error {
+		for _, chart := range HelmCharts {
+			prefix := "chart." + chart.ReleaseName + "."
+			legacy, err := common.PropertyGetAllByPrefix("scheduler-k3s", "--global", prefix)
+			if err != nil {
+				return fmt.Errorf("Unable to read legacy chart properties for %s: %w", chart.ReleaseName, err)
+			}
+			if len(legacy) == 0 {
+				continue
+			}
 
-		mapProperty := "chart-overrides." + chart.ReleaseName
-		merged, err := common.PropertyMapGet("scheduler-k3s", "--global", mapProperty)
-		if err != nil {
-			return fmt.Errorf("Unable to read existing chart-overrides for %s: %w", chart.ReleaseName, err)
-		}
-		for fullKey, value := range legacy {
-			merged[strings.TrimPrefix(fullKey, prefix)] = value
-		}
+			mapProperty := "chart-overrides." + chart.ReleaseName
+			merged, err := common.PropertyMapGet("scheduler-k3s", "--global", mapProperty)
+			if err != nil {
+				return fmt.Errorf("Unable to read existing chart-overrides for %s: %w", chart.ReleaseName, err)
+			}
+			for fullKey, value := range legacy {
+				merged[strings.TrimPrefix(fullKey, prefix)] = value
+			}
 
-		if err := common.PropertyMapWrite("scheduler-k3s", "--global", mapProperty, merged); err != nil {
-			return fmt.Errorf("Unable to write chart-overrides for %s: %w", chart.ReleaseName, err)
-		}
+			if err := p.MapWrite(mapProperty, merged); err != nil {
+				return fmt.Errorf("Unable to write chart-overrides for %s: %w", chart.ReleaseName, err)
+			}
 
-		for fullKey := range legacy {
-			if err := common.PropertyDelete("scheduler-k3s", "--global", fullKey); err != nil {
-				return fmt.Errorf("Unable to remove legacy chart property %s: %w", fullKey, err)
+			for fullKey := range legacy {
+				if err := p.Delete(fullKey); err != nil {
+					return fmt.Errorf("Unable to remove legacy chart property %s: %w", fullKey, err)
+				}
 			}
 		}
-	}
-	return nil
+		return nil
+	})
 }
 
 // migrateAnnotationsLabelsToMapFormat converts the legacy line-formatted
@@ -207,31 +209,33 @@ func migrateAnnotationsLabelsToMapFormat() error {
 // file as a JSON map. It is a no-op when the file is already valid JSON or
 // empty (idempotent).
 func migrateAnnotationLabelProperty(scope string, propertyName string) error {
-	if _, err := common.PropertyMapGet("scheduler-k3s", scope, propertyName); err == nil {
+	return common.PropertyLock("scheduler-k3s", scope, func(p *common.LockedProperties) error {
+		if _, err := common.PropertyMapGet("scheduler-k3s", scope, propertyName); err == nil {
+			return nil
+		}
+
+		lines, err := common.PropertyListGet("scheduler-k3s", scope, propertyName)
+		if err != nil {
+			return fmt.Errorf("Unable to read legacy property %s/%s: %w", scope, propertyName, err)
+		}
+
+		m := map[string]string{}
+		for _, line := range lines {
+			if line == "" {
+				continue
+			}
+			parts := strings.SplitN(line, ": ", 2)
+			if len(parts) != 2 {
+				return fmt.Errorf("Invalid legacy entry in %s/%s: %s", scope, propertyName, line)
+			}
+			m[parts[0]] = parts[1]
+		}
+
+		if err := p.MapWrite(propertyName, m); err != nil {
+			return fmt.Errorf("Unable to write %s/%s as map: %w", scope, propertyName, err)
+		}
 		return nil
-	}
-
-	lines, err := common.PropertyListGet("scheduler-k3s", scope, propertyName)
-	if err != nil {
-		return fmt.Errorf("Unable to read legacy property %s/%s: %w", scope, propertyName, err)
-	}
-
-	m := map[string]string{}
-	for _, line := range lines {
-		if line == "" {
-			continue
-		}
-		parts := strings.SplitN(line, ": ", 2)
-		if len(parts) != 2 {
-			return fmt.Errorf("Invalid legacy entry in %s/%s: %s", scope, propertyName, line)
-		}
-		m[parts[0]] = parts[1]
-	}
-
-	if err := common.PropertyMapWrite("scheduler-k3s", scope, propertyName, m); err != nil {
-		return fmt.Errorf("Unable to write %s/%s as map: %w", scope, propertyName, err)
-	}
-	return nil
+	})
 }
 
 // TriggerPostCertsUpdate handles post-certs-update trigger

plugins/storage/attachment.go

@@ -105,6 +105,11 @@ func LoadAttachments(appName string) ([]*Attachment, error) {
 		return nil, fmt.Errorf("unable to read storage attachments for %q: %w", appName, err)
 	}
 
+	return decodeAttachments(appName, lines)
+}
+
+// decodeAttachments parses the stored attachment lines of an app.
+func decodeAttachments(appName string, lines []string) ([]*Attachment, error) {
 	attachments := []*Attachment{}
 	for _, line := range lines {
 		line = strings.TrimSpace(line)
@@ -122,15 +127,44 @@ func LoadAttachments(appName string) ([]*Attachment, error) {
 
 // SaveAttachments overwrites the entire attachment list for an app.
 func SaveAttachments(appName string, attachments []*Attachment) error {
+	lines, err := encodeAttachments(appName, attachments)
+	if err != nil {
+		return err
+	}
+	return common.PropertyListWrite(PluginName, appName, AttachmentsProperty, lines)
+}
+
+// UpdateAttachments loads the attachment list for an app, passes it to fn and
+// saves the list fn returns, holding the app's storage property lock across
+// the read and the write so concurrent mounts and unmounts cannot drop each
+// other's changes. Nothing is saved when fn returns an error.
+func UpdateAttachments(appName string, fn func(attachments []*Attachment) ([]*Attachment, error)) error {
+	return common.PropertyListUpdate(PluginName, appName, AttachmentsProperty, func(lines []string) ([]string, error) {
+		attachments, err := decodeAttachments(appName, lines)
+		if err != nil {
+			return nil, err
+		}
+
+		attachments, err = fn(attachments)
+		if err != nil {
+			return nil, err
+		}
+
+		return encodeAttachments(appName, attachments)
+	})
+}
+
+// encodeAttachments serializes attachments into stored attachment lines.
+func encodeAttachments(appName string, attachments []*Attachment) ([]string, error) {
 	lines := make([]string, 0, len(attachments))
 	for _, attachment := range attachments {
 		data, err := json.Marshal(attachment)
 		if err != nil {
-			return fmt.Errorf("unable to encode storage attachment for %q: %w", appName, err)
+			return nil, fmt.Errorf("unable to encode storage attachment for %q: %w", appName, err)
 		}
 		lines = append(lines, string(data))
 	}
-	return common.PropertyListWrite(PluginName, appName, AttachmentsProperty, lines)
+	return lines, nil
 }
 
 // ensureContainerPathFree rejects a candidate whose container path is already
@@ -161,26 +195,22 @@ func AddAttachment(appName string, attachment *Attachment) error {
 		return err
 	}
 
-	attachments, err := LoadAttachments(appName)
-	if err != nil {
-		return err
-	}
-
-	for _, existing := range attachments {
-		if existing.EntryName == attachment.EntryName &&
-			existing.ContainerPath == attachment.ContainerPath &&
-			existing.EffectiveProcessType() == attachment.EffectiveProcessType() {
-			return fmt.Errorf("storage entry %q is already mounted at %q for process type %q on app %q",
-				attachment.EntryName, attachment.ContainerPath, attachment.EffectiveProcessType(), appName)
+	return UpdateAttachments(appName, func(attachments []*Attachment) ([]*Attachment, error) {
+		for _, existing := range attachments {
+			if existing.EntryName == attachment.EntryName &&
+				existing.ContainerPath == attachment.ContainerPath &&
+				existing.EffectiveProcessType() == attachment.EffectiveProcessType() {
+				return nil, fmt.Errorf("storage entry %q is already mounted at %q for process type %q on app %q",
+					attachment.EntryName, attachment.ContainerPath, attachment.EffectiveProcessType(), appName)
+			}
 		}
-	}
 
-	if err := ensureContainerPathFree(appName, attachments, attachment); err != nil {
-		return err
-	}
+		if err := ensureContainerPathFree(appName, attachments, attachment); err != nil {
+			return nil, err
+		}
 
-	attachments = append(attachments, attachment)
-	return SaveAttachments(appName, attachments)
+		return append(attachments, attachment), nil
+	})
 }
 
 // UpsertAttachment inserts an attachment, or updates the mount-time fields
@@ -201,49 +231,43 @@ func UpsertAttachment(appName string, attachment *Attachment) (bool, error) {
 		return false, err
 	}
 
-	attachments, err := LoadAttachments(appName)
+	created := false
+	err := UpdateAttachments(appName, func(attachments []*Attachment) ([]*Attachment, error) {
+		for _, existing := range attachments {
+			if existing.EntryName == attachment.EntryName &&
+				existing.ContainerPath == attachment.ContainerPath &&
+				existing.EffectiveProcessType() == attachment.EffectiveProcessType() {
+				existing.Phases = attachment.Phases
+				existing.Subpath = attachment.Subpath
+				existing.Readonly = attachment.Readonly
+				existing.VolumeOptions = attachment.VolumeOptions
+				existing.VolumeChown = attachment.VolumeChown
+				return attachments, nil
+			}
+		}
+
+		// Only the append branch needs the overlap check: rewriting an existing
+		// tuple in place would otherwise trip over the attachment it is updating.
+		if err := ensureContainerPathFree(appName, attachments, attachment); err != nil {
+			return nil, err
+		}
+
+		created = true
+		return append(attachments, attachment), nil
+	})
 	if err != nil {
 		return false, err
 	}
-
-	for _, existing := range attachments {
-		if existing.EntryName == attachment.EntryName &&
-			existing.ContainerPath == attachment.ContainerPath &&
-			existing.EffectiveProcessType() == attachment.EffectiveProcessType() {
-			existing.Phases = attachment.Phases
-			existing.Subpath = attachment.Subpath
-			existing.Readonly = attachment.Readonly
-			existing.VolumeOptions = attachment.VolumeOptions
-			existing.VolumeChown = attachment.VolumeChown
-			return false, SaveAttachments(appName, attachments)
-		}
-	}
-
-	// Only the append branch needs the overlap check: rewriting an existing
-	// tuple in place would otherwise trip over the attachment it is updating.
-	if err := ensureContainerPathFree(appName, attachments, attachment); err != nil {
-		return false, err
-	}
-
-	attachments = append(attachments, attachment)
-	return true, SaveAttachments(appName, attachments)
+	return created, nil
 }
 
 // RemoveAttachment removes the attachment matching the given entry and
 // optional container path. If containerPath is empty and there is more
 // than one match, returns an error.
 func RemoveAttachment(appName string, entryName string, containerPath string) error {
-	attachments, err := LoadAttachments(appName)
-	if err != nil {
-		return err
-	}
-
-	keep, err := removeMatchingAttachments(attachments, appName, entryName, containerPath)
-	if err != nil {
-		return err
-	}
-
-	return SaveAttachments(appName, keep)
+	return UpdateAttachments(appName, func(attachments []*Attachment) ([]*Attachment, error) {
+		return removeMatchingAttachments(attachments, appName, entryName, containerPath)
+	})
 }
 
 // removeMatchingAttachments returns the given list with the attachments

plugins/storage/commands_entries.go

@@ -274,32 +274,27 @@ func CommandSet(input CommandSetInput) error {
 	if !EntryExists(input.Name) {
 		return fmt.Errorf("storage entry %q does not exist", input.Name)
 	}
-	entry, err := LoadEntry(input.Name)
-	if err != nil {
-		return err
-	}
-
 	touchesDirectory := false
-	for _, change := range input.Changes {
-		if err := applyPropertyChange(entry, change); err != nil {
-			return err
+	entry, err := UpdateEntry(input.Name, func(entry *Entry) error {
+		for _, change := range input.Changes {
+			if err := applyPropertyChange(entry, change); err != nil {
+				return err
+			}
+			if change.Property == "chown" || change.Property == "mode" {
+				touchesDirectory = true
+			}
 		}
-		if change.Property == "chown" || change.Property == "mode" {
-			touchesDirectory = true
+
+		if input.Annotations != nil {
+			entry.Annotations = input.Annotations
+		}
+		if input.Labels != nil {
+			entry.Labels = input.Labels
 		}
-	}
 
-	if input.Annotations != nil {
-		entry.Annotations = input.Annotations
-	}
-	if input.Labels != nil {
-		entry.Labels = input.Labels
-	}
-
-	if err := entry.Validate(); err != nil {
-		return err
-	}
-	if err := SaveEntry(entry); err != nil {
+		return entry.Validate()
+	})
+	if err != nil {
 		return err
 	}
 	// Only converge the directory when the caller actually asked to change

plugins/storage/commands_metadata.go

@@ -76,32 +76,27 @@ func setEntryMapKey(field entryMapField, name string, key string, value string)
 		return fmt.Errorf("storage entry %q does not exist", name)
 	}
 
-	entry, err := LoadEntry(name)
-	if err != nil {
-		return err
-	}
-
-	values := field.Get(entry)
-	if value == "" {
-		delete(values, key)
-		common.LogInfo2Quiet(fmt.Sprintf("Unsetting %s %s", field.Singular, key))
-	} else {
-		if values == nil {
-			values = map[string]string{}
+	entry, err := UpdateEntry(name, func(entry *Entry) error {
+		values := field.Get(entry)
+		if value == "" {
+			delete(values, key)
+			common.LogInfo2Quiet(fmt.Sprintf("Unsetting %s %s", field.Singular, key))
+		} else {
+			if values == nil {
+				values = map[string]string{}
+			}
+			values[key] = value
+			common.LogInfo2Quiet(fmt.Sprintf("Setting %s %s to %s", field.Singular, key, value))
 		}
-		values[key] = value
-		common.LogInfo2Quiet(fmt.Sprintf("Setting %s %s to %s", field.Singular, key, value))
-	}
-	if len(values) == 0 {
-		// Drop the empty map so the omitempty tag keeps it out of the JSON.
-		values = nil
-	}
-	field.Set(entry, values)
+		if len(values) == 0 {
+			// Drop the empty map so the omitempty tag keeps it out of the JSON.
+			values = nil
+		}
+		field.Set(entry, values)
 
-	if err := entry.Validate(); err != nil {
-		return err
-	}
-	if err := SaveEntry(entry); err != nil {
+		return entry.Validate()
+	})
+	if err != nil {
 		return err
 	}
 

plugins/storage/concurrency_test.go

@@ -0,0 +1,92 @@
+package storage
+
+import (
+	"errors"
+	"fmt"
+	"sync"
+	"testing"
+
+	. "github.com/onsi/gomega"
+)
+
+func TestAddAttachmentConcurrent(t *testing.T) {
+	RegisterTestingT(t)
+	withTempLibRoot(t)
+
+	const count = 20
+	var wg sync.WaitGroup
+	errs := make([]error, count)
+	for i := 0; i < count; i++ {
+		wg.Add(1)
+		go func(i int) {
+			defer wg.Done()
+			errs[i] = AddAttachment("demo", &Attachment{
+				EntryName:     fmt.Sprintf("demo-data-%d", i),
+				ContainerPath: fmt.Sprintf("/data/%d", i),
+				Phases:        []string{PhaseDeploy, PhaseRun},
+				ProcessType:   DefaultProcessType,
+			})
+		}(i)
+	}
+	wg.Wait()
+	Expect(errors.Join(errs...)).NotTo(HaveOccurred())
+
+	attachments, err := LoadAttachments("demo")
+	Expect(err).NotTo(HaveOccurred())
+	Expect(attachments).To(HaveLen(count))
+}
+
+func TestUpdateEntryConcurrent(t *testing.T) {
+	RegisterTestingT(t)
+	withTempLibRoot(t)
+
+	Expect(SaveEntry(&Entry{
+		Name:      "demo-data",
+		Scheduler: SchedulerK3s,
+		Size:      "2Gi",
+	})).To(Succeed())
+
+	const count = 20
+	var wg sync.WaitGroup
+	errs := make([]error, count)
+	for i := 0; i < count; i++ {
+		wg.Add(1)
+		go func(i int) {
+			defer wg.Done()
+			_, errs[i] = UpdateEntry("demo-data", func(entry *Entry) error {
+				if entry.Labels == nil {
+					entry.Labels = map[string]string{}
+				}
+				entry.Labels[fmt.Sprintf("label-%d", i)] = "value"
+				return nil
+			})
+		}(i)
+	}
+	wg.Wait()
+	Expect(errors.Join(errs...)).NotTo(HaveOccurred())
+
+	entry, err := LoadEntry("demo-data")
+	Expect(err).NotTo(HaveOccurred())
+	Expect(entry.Labels).To(HaveLen(count))
+}
+
+func TestUpdateEntryErrorLeavesEntryUnchanged(t *testing.T) {
+	RegisterTestingT(t)
+	withTempLibRoot(t)
+
+	Expect(SaveEntry(&Entry{
+		Name:      "demo-data",
+		Scheduler: SchedulerK3s,
+		Size:      "2Gi",
+	})).To(Succeed())
+
+	_, err := UpdateEntry("demo-data", func(entry *Entry) error {
+		entry.Size = "5Gi"
+		return errors.New("boom")
+	})
+	Expect(err).To(MatchError("boom"))
+
+	entry, err := LoadEntry("demo-data")
+	Expect(err).NotTo(HaveOccurred())
+	Expect(entry.Size).To(Equal("2Gi"))
+}

plugins/storage/entry.go

@@ -67,7 +67,7 @@ var (
 )
 
 // Entry is the source of truth for a storage volume. One file per entry
-// lives at $DOKKU_LIB_ROOT/config/storage/entries/<name>.json.
+// lives at $DOKKU_LIB_ROOT/data/storage-registry/entries/<name>.json.
 type Entry struct {
 	Name          string            `json:"name"`
 	Scheduler     string            `json:"scheduler"`
@@ -162,6 +162,39 @@ func SaveEntry(entry *Entry) error {
 	return nil
 }
 
+// UpdateEntry loads a registered entry, passes it to fn and saves it, holding
+// the entries directory lock across the read and the write so concurrent
+// edits to one entry cannot drop each other's changes. Nothing is saved when
+// fn returns an error. The saved entry is returned.
+func UpdateEntry(name string, fn func(entry *Entry) error) (*Entry, error) {
+	if err := EnsureEntriesDirectory(); err != nil {
+		return nil, err
+	}
+
+	var updated *Entry
+	err := common.LockDirectory(EntriesDirectory(), func() error {
+		entry, err := LoadEntry(name)
+		if err != nil {
+			return err
+		}
+
+		if err := fn(entry); err != nil {
+			return err
+		}
+
+		if err := SaveEntry(entry); err != nil {
+			return err
+		}
+
+		updated = entry
+		return nil
+	})
+	if err != nil {
+		return nil, err
+	}
+	return updated, nil
+}
+
 // DeleteEntry removes the on-disk record for an entry.
 func DeleteEntry(name string) error {
 	if err := os.Remove(entryPath(name)); err != nil && !os.IsNotExist(err) {

plugins/storage/migrate.go

@@ -217,19 +217,18 @@ func migrateMount(appName string, processType string, mount string, phases []str
 		VolumeOptions: parsed.VolumeOptions,
 	}
 
-	existing, err := LoadAttachments(appName)
-	if err != nil {
-		return err
-	}
 	// A named scope whose mount the default scope already covers needs no
 	// attachment of its own - the default one reaches that process anyway, and
 	// recording both would manufacture two -v flags for one container path.
 	// The docker-options line is still drained so it stops being emitted twice.
-	if !attachmentExists(existing, attachment) && !coveredByDefaultScope(existing, attachment) {
-		existing = append(existing, attachment)
-		if err := SaveAttachments(appName, existing); err != nil {
-			return err
+	err := UpdateAttachments(appName, func(existing []*Attachment) ([]*Attachment, error) {
+		if attachmentExists(existing, attachment) || coveredByDefaultScope(existing, attachment) {
+			return existing, nil
 		}
+		return append(existing, attachment), nil
+	})
+	if err != nil {
+		return err
 	}
 
 	mountedPhases := []string{}

plugins/storage/subcommands.go

@@ -390,34 +390,30 @@ func replaceMounts(input CommandMountInput) error {
 		}
 	}
 
-	existing, err := LoadAttachments(input.AppName)
+	err := UpdateAttachments(input.AppName, func(existing []*Attachment) ([]*Attachment, error) {
+		retained := []*Attachment{}
+		for _, attachment := range existing {
+			if attachment.EffectiveProcessType() != processType {
+				retained = append(retained, attachment)
+			}
+		}
+
+		// The declared set is already unique on container path within its own
+		// scope; what is left to check is the scopes this call does not replace,
+		// so a declared path cannot collide with one held by another process type.
+		// Checking before the write keeps the all-or-nothing contract.
+		for _, attachment := range declared {
+			if err := ensureContainerPathFree(input.AppName, retained, attachment); err != nil {
+				return nil, err
+			}
+		}
+
+		return append(retained, declared...), nil
+	})
 	if err != nil {
 		return err
 	}
 
-	retained := []*Attachment{}
-	for _, attachment := range existing {
-		if attachment.EffectiveProcessType() != processType {
-			retained = append(retained, attachment)
-		}
-	}
-
-	// The declared set is already unique on container path within its own
-	// scope; what is left to check is the scopes this call does not replace,
-	// so a declared path cannot collide with one held by another process type.
-	// Checking before the write keeps the all-or-nothing contract.
-	for _, attachment := range declared {
-		if err := ensureContainerPathFree(input.AppName, retained, attachment); err != nil {
-			return err
-		}
-	}
-
-	attachments := append(retained, declared...)
-
-	if err := SaveAttachments(input.AppName, attachments); err != nil {
-		return err
-	}
-
 	common.LogInfo1(fmt.Sprintf("Storage mounts for process type %s replaced on %s", processType, input.AppName))
 	return nil
 }
@@ -510,30 +506,27 @@ func resolveUnmountTarget(mount string, containerDir string) (unmountTarget, err
 // written, so an argument naming a mount the app does not have leaves the rest
 // of the set in place rather than removing a prefix of it.
 func unmountMounts(input CommandUnmountInput) error {
-	attachments, err := LoadAttachments(input.AppName)
-	if err != nil {
-		return err
-	}
-
-	for _, mount := range input.Mounts {
-		target, err := resolveUnmountTarget(mount, input.ContainerDir)
-		if err != nil {
-			return err
-		}
-
-		attachments, err = removeMatchingAttachments(attachments, input.AppName, target.EntryName, target.ContainerPath)
-		if err != nil {
-			// The legacy form historically said "Mount path does not
-			// exist." - preserve that exact wording so existing automation
-			// and the bats suite keep matching.
-			if target.Legacy && strings.Contains(err.Error(), "is not mounted") {
-				return errors.New("Mount path does not exist.")
+	return UpdateAttachments(input.AppName, func(attachments []*Attachment) ([]*Attachment, error) {
+		for _, mount := range input.Mounts {
+			target, err := resolveUnmountTarget(mount, input.ContainerDir)
+			if err != nil {
+				return nil, err
 			}
-			return err
-		}
-	}
 
-	return SaveAttachments(input.AppName, attachments)
+			attachments, err = removeMatchingAttachments(attachments, input.AppName, target.EntryName, target.ContainerPath)
+			if err != nil {
+				// The legacy form historically said "Mount path does not
+				// exist." - preserve that exact wording so existing automation
+				// and the bats suite keep matching.
+				if target.Legacy && strings.Contains(err.Error(), "is not mounted") {
+					return nil, errors.New("Mount path does not exist.")
+				}
+				return nil, err
+			}
+		}
+
+		return attachments, nil
+	})
 }
 
 // unmountAll removes every attachment on an app, or every attachment for one
@@ -560,19 +553,16 @@ func unmountAll(input CommandUnmountInput) error {
 		return nil
 	}
 
-	attachments, err := LoadAttachments(input.AppName)
-	if err != nil {
-		return err
-	}
-
-	keep := []*Attachment{}
-	for _, attachment := range attachments {
-		if attachment.EffectiveProcessType() != input.ProcessType {
-			keep = append(keep, attachment)
+	err := UpdateAttachments(input.AppName, func(attachments []*Attachment) ([]*Attachment, error) {
+		keep := []*Attachment{}
+		for _, attachment := range attachments {
+			if attachment.EffectiveProcessType() != input.ProcessType {
+				keep = append(keep, attachment)
+			}
 		}
-	}
-
-	if err := SaveAttachments(input.AppName, keep); err != nil {
+		return keep, nil
+	})
+	if err != nil {
 		return err
 	}
 

tests/unit/buildpacks.bats

@@ -476,6 +476,29 @@ teardown() {
   assert_output_contains "heroku-buildpack-python"
 }
 
+@test "(buildpacks:add) concurrent calls for the same app keep every buildpack" {
+  run run_concurrently 20 /bin/bash -c 'dokku buildpacks:add "$1" "https://github.com/dokku/buildpack-race$2.git"' _ "$TEST_APP"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "dokku --quiet buildpacks:list $TEST_APP | grep -cE '^ *https://github.com/dokku/buildpack-race[0-9]+\.git$'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+  assert_output "20"
+
+  run run_concurrently 20 /bin/bash -c 'dokku buildpacks:remove "$1" "https://github.com/dokku/buildpack-race$2.git"' _ "$TEST_APP"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "dokku --quiet buildpacks:list $TEST_APP | grep -cE '^ *https://github.com/dokku/buildpack-race[0-9]+\.git$'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_output "0"
+}
+
 template_buildpacks_appjson() {
   local APP="$1"
   local APP_REPO_DIR="$2"

tests/unit/caddy.bats

@@ -429,3 +429,26 @@ teardown() {
   assert_success
   assert_output_not_exists
 }
+
+@test "(caddy:labels:add) concurrent calls for the same app keep every label" {
+  run run_concurrently 20 /bin/bash -c 'dokku caddy:labels:add "$1" "caddy.race$2" value' _ "$TEST_APP"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "dokku caddy:labels:show $TEST_APP | grep -c 'caddy\.race[0-9]*=value'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+  assert_output "20"
+
+  run run_concurrently 20 /bin/bash -c 'dokku caddy:labels:remove "$1" "caddy.race$2"' _ "$TEST_APP"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "dokku caddy:labels:show $TEST_APP 2>/dev/null | grep -c 'caddy\.race[0-9]*=value'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_output "0"
+}

tests/unit/checks.bats

@@ -374,3 +374,26 @@ teardown() {
   assert_output_contains "Failure in name='port listening check': container listening on expected IPv4 interface with an unexpected port: expected=5000 actual=5001"
   assert_output_contains "Running healthcheck name='port listening check' attempts=3 port=5000 retries=2 timeout=5 type='listening' wait=5"
 }
+
+@test "(checks:disable) concurrent calls for the same app keep every process type" {
+  run run_concurrently 20 /bin/bash -c 'dokku checks:disable "$1" "worker$2"' _ "$TEST_APP"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "dokku checks:report $TEST_APP --checks-disabled-list | tr ',' '\n' | grep -cx 'worker[0-9]*'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+  assert_output "20"
+
+  run run_concurrently 20 /bin/bash -c 'dokku checks:enable "$1" "worker$2"' _ "$TEST_APP"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "dokku checks:report $TEST_APP --checks-disabled-list | tr ',' '\n' | grep -cx 'worker[0-9]*'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_output "0"
+}

tests/unit/config.bats

@@ -787,6 +787,29 @@ teardown() {
   assert_output_contains "Invalid key name"
 }
 
+@test "(config:set) concurrent calls for the same app keep every key" {
+  run run_concurrently 20 /bin/bash -c 'dokku config:set --no-restart "$1" "RACE_KEY_$2=value"' _ "$TEST_APP"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "dokku config:keys $TEST_APP | grep -cx 'RACE_KEY_[0-9]*'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+  assert_output "20"
+
+  run run_concurrently 20 /bin/bash -c 'dokku config:unset --no-restart "$1" "RACE_KEY_$2"' _ "$TEST_APP"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "dokku config:keys $TEST_APP | grep -cx 'RACE_KEY_[0-9]*'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_output "0"
+}
+
 block_app_config_dir() {
   declare desc="makes the app config directory unwritable for the dokku user"
 

tests/unit/docker-options-2.bats

@@ -316,6 +316,29 @@ teardown() {
   assert_output_contains "running"
 }
 
+@test "(docker-options:add) concurrent calls for the same app keep every option" {
+  run run_concurrently 20 /bin/bash -c 'dokku docker-options:add "$1" deploy "--label=race$2"' _ "$TEST_APP"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "dokku docker-options:list $TEST_APP --phase deploy | grep -cx -- '--label=race[0-9]*'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+  assert_output "20"
+
+  run run_concurrently 20 /bin/bash -c 'dokku docker-options:remove "$1" deploy "--label=race$2"' _ "$TEST_APP"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "dokku docker-options:list $TEST_APP --phase deploy | grep -cx -- '--label=race[0-9]*'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_output "0"
+}
+
 web_worker_callback() {
   local APP="$1"
   local APP_REPO_DIR="$2"

tests/unit/ports.bats

@@ -415,3 +415,26 @@ teardown() {
   assert_success
   assert_output "true"
 }
+
+@test "(ports:add) concurrent calls for the same app keep every port mapping" {
+  run run_concurrently 20 /bin/bash -c 'dokku ports:add "$1" "http:$((8000 + $2)):5000"' _ "$TEST_APP"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "dokku --quiet ports:report $TEST_APP --ports-map | tr ' ' '\n' | grep -cx 'http:80[0-9][0-9]:5000'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+  assert_output "20"
+
+  run run_concurrently 20 /bin/bash -c 'dokku ports:remove "$1" "http:$((8000 + $2)):5000"' _ "$TEST_APP"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "dokku --quiet ports:report $TEST_APP --ports-map | tr ' ' '\n' | grep -cx 'http:80[0-9][0-9]:5000'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_output "0"
+}

tests/unit/storage-3.bats

@@ -488,3 +488,53 @@ teardown() {
   run /bin/bash -c "dokku storage:destroy rdmtest-cache --force"
   assert_success
 }
+
+@test "(storage:mount) concurrent calls for the same app keep every mount" {
+  run run_concurrently 20 /bin/bash -c 'dokku storage:create "rdmtest-race$1"' _
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run run_concurrently 20 /bin/bash -c 'dokku storage:mount "$1" "rdmtest-race$2" --container-dir "/race/$2"' _ "$TEST_APP"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "dokku storage:list $TEST_APP --format json | jq -r '. | length'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+  assert_output "20"
+
+  run run_concurrently 20 /bin/bash -c 'dokku storage:unmount "$1" "rdmtest-race$2"' _ "$TEST_APP"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "dokku storage:list $TEST_APP --format json | jq -r '. | length'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_output "0"
+
+  run run_concurrently 20 /bin/bash -c 'dokku storage:destroy "rdmtest-race$1" --force' _
+  assert_success
+}
+
+@test "(storage:labels:set) concurrent calls for the same entry keep every label" {
+  run /bin/bash -c "dokku storage:create rdmtest-race-label"
+  assert_success
+
+  run run_concurrently 20 /bin/bash -c 'dokku storage:labels:set rdmtest-race-label "example.com/race$1" value' _
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "dokku storage:info rdmtest-race-label --format json | jq -r '.labels | length'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+  assert_output "20"
+
+  run /bin/bash -c "dokku storage:destroy rdmtest-race-label --force"
+  assert_success
+}

tests/unit/test_helper.bash

@@ -756,3 +756,23 @@ uninstall_k3s() {
   echo "status: $status"
   assert_success
 }
+
+run_concurrently() {
+  declare desc="runs a command COUNT times in parallel, appending the run number as the last argument, and fails if any run fails"
+  declare COUNT="$1"
+  shift 1
+  local i pid failed=0
+  local pids=()
+
+  for i in $(seq 1 "$COUNT"); do
+    "$@" "$i" &
+    pids+=("$!")
+  done
+
+  for pid in "${pids[@]}"; do
+    wait "$pid" || failed=$((failed + 1))
+  done
+
+  echo "failed runs: $failed"
+  [[ "$failed" -eq 0 ]]
+}