Merge pull request #9172 from dokku/9107-concurrent-docker-options-add-calls-for-the-same-app-lose-options

Serialize concurrent property updates

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 ]]
+}