Concurrency hardening, commit rollback, and CI bumps

.claude/rules/build-commands.md

@@ -52,7 +52,7 @@ Environment variables: `OCTI_PORT`, `OCTI_DEBUG`, `OCTI_DATA_DIR`
 
 - **code-checks.yml**: runs `assemble` + `check` on push to main and PRs
 - **release-tag.yml**: builds `installDist`, zips, creates GitHub release on `v*` tags
-- JDK 17 (adopt), Gradle 8.13
+- JDK 21 (temurin), Gradle 9.4.1
 
 ## Context Management
 

.github/actions/common-setup/action.yml

@@ -4,17 +4,17 @@ runs:
   using: "composite"
   steps:
     - name: Set up JDK 21
-      uses: actions/setup-java@17f84c3641ba7b8f6deff6309fc4c864478f5d62 #v3
+      uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654 #v5.2.0
       with:
         java-version: '21'
-        distribution: 'adopt'
+        distribution: 'temurin'
 
     - name: Grant execute permission for gradlew
       shell: bash
       run: chmod +x gradlew
 
     - name: Cache Gradle Wrapper
-      uses: actions/cache@6f8efc29b200d32929f49075959781ed54ec270c #v3
+      uses: actions/cache@cdf6c1fa76f9f475f3d7449005a359c84ca0f306 #v5.0.3
       with:
         path: |
           ~/.gradle/wrapper
@@ -24,7 +24,7 @@ runs:
           ${{ runner.os }}-gradle-wrapper-
 
     - name: Cache Gradle Dependencies
-      uses: actions/cache@6f8efc29b200d32929f49075959781ed54ec270c #v3
+      uses: actions/cache@cdf6c1fa76f9f475f3d7449005a359c84ca0f306 #v5.0.3
       with:
         path: |
           ~/.gradle/caches

.github/workflows/code-checks.yml

@@ -15,7 +15,7 @@ jobs:
     runs-on: ubuntu-24.04
     steps:
       - name: Checkout source code
-        uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 #v4
+        uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd #v6.0.2
         with:
           persist-credentials: false
       - name: Setup project and build environment
@@ -28,7 +28,7 @@ jobs:
     runs-on: ubuntu-24.04
     steps:
       - name: Checkout source code
-        uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 #v4
+        uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd #v6.0.2
         with:
           persist-credentials: false
       - name: Set up Docker Buildx
@@ -53,7 +53,7 @@ jobs:
       LC_ALL: C.UTF-8
     steps:
       - name: Checkout source code
-        uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 #v4
+        uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd #v6.0.2
         with:
           persist-credentials: false
       - name: Setup project and build environment
@@ -67,7 +67,7 @@ jobs:
     needs: [run-tests, build-docker-image]
     steps:
       - name: Checkout source code
-        uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 #v4
+        uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd #v6.0.2
         with:
           persist-credentials: false
       - name: Setup project and build environment

.github/workflows/release-tag.yml

@@ -17,22 +17,14 @@ jobs:
     environment: foss-production
     steps:
       - name: Checkout source code
-        uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 #v4
+        uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd #v6.0.2
         with:
           fetch-depth: 0
           persist-credentials: false
 
       - name: Setup project and build environment
         uses: ./.github/actions/common-setup
 
-      - name: Get the version
-        id: tagger
-        uses: jimschubert/query-tag-action@ebe504c35d1ea44d7272c8eccc7a71e2e4f1b1cb #v2
-        with:
-          skip-unshallow: 'true'
-          abbrev: false
-          commit-ish: HEAD
-
       - name: Set up QEMU
         uses: docker/setup-qemu-action@ce360397dd3f832beb865e1373c09c0e9f86d70a #v4.0.0
 
@@ -76,25 +68,12 @@ jobs:
       - name: Create ZIP file from the directory
         run: zip -r octi-server.zip ./build/install/octi-server
 
-      - name: Create pre-release
-        if: contains(steps.tagger.outputs.tag, '-')
-        uses: softprops/action-gh-release@de2c0eb89ae2a093876385947365aca7b0e5f844 #v1
-        with:
-          prerelease: true
-          tag_name: ${{ steps.tagger.outputs.tag }}
-          name: ${{ steps.tagger.outputs.tag }}
-          generate_release_notes: true
-          files: octi-server.zip
-        env:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
-
-      - name: Create release
-        if: "!contains(steps.tagger.outputs.tag, '-')"
-        uses: softprops/action-gh-release@de2c0eb89ae2a093876385947365aca7b0e5f844 #v1
+      - name: Create GitHub release
+        uses: softprops/action-gh-release@b4309332981a82ec1c5618f44dd2e27cc8bfbfda #v3.0.0
         with:
-          prerelease: false
-          tag_name: ${{ steps.tagger.outputs.tag }}
-          name: ${{ steps.tagger.outputs.tag }}
+          prerelease: ${{ contains(github.ref_name, '-beta') }}
+          tag_name: ${{ github.ref_name }}
+          name: ${{ github.ref_name }}
           generate_release_notes: true
           files: octi-server.zip
         env:

build.gradle.kts

@@ -1,7 +1,7 @@
 plugins {
     kotlin("jvm") version "2.3.10"
     application
-    id("com.google.devtools.ksp") version "2.3.5"
+    id("com.google.devtools.ksp") version "2.3.6"
     kotlin("plugin.serialization") version "2.3.10"
     id("io.ktor.plugin") version "3.4.0"
 }
@@ -14,8 +14,8 @@ repositories {
 }
 
 dependencies {
-    implementation("com.google.dagger:dagger:2.59.1")
-    ksp("com.google.dagger:dagger-compiler:2.59.1")
+    implementation("com.google.dagger:dagger:2.59.2")
+    ksp("com.google.dagger:dagger-compiler:2.59.2")
 
     val ktorVersion = "3.4.0"
     implementation("io.ktor:ktor-server-core:$ktorVersion")

src/main/kotlin/eu/darken/octi/server/device/DeviceRepo.kt

@@ -11,9 +11,12 @@ import eu.darken.octi.server.common.debug.logging.Logging.Priority.*
 import eu.darken.octi.server.common.debug.logging.asLog
 import eu.darken.octi.server.common.debug.logging.log
 import eu.darken.octi.server.common.debug.logging.logTag
+import kotlinx.coroutines.CompletableDeferred
+import kotlinx.coroutines.NonCancellable
 import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.sync.Mutex
 import kotlinx.coroutines.sync.withLock
+import kotlinx.coroutines.withContext
 import kotlinx.serialization.json.Json
 import java.io.IOException
 import java.time.Duration
@@ -36,6 +39,8 @@ class DeviceRepo @Inject constructor(
 
     private val devices = ConcurrentHashMap<DeviceKey, Device>()
     private val lastSeenPersistedAt = ConcurrentHashMap<DeviceKey, Instant>()
+    private val deletingDevices = mutableMapOf<DeviceKey, PendingDeviceDeletion>()
+    private val deletingDeviceIds = mutableMapOf<DeviceId, PendingDeviceDeletion>()
     private val mutex = Mutex()
 
     init {
@@ -108,6 +113,28 @@ class DeviceRepo @Inject constructor(
         path.resolve(DEVICE_FILENAME).writeText(serializer.encodeToString(data))
     }
 
+    private data class PendingDeviceDeletion(
+        val device: Device,
+        val completed: CompletableDeferred<Unit> = CompletableDeferred(),
+    )
+
+    private fun trackPendingDeletion(device: Device): PendingDeviceDeletion {
+        val pendingDeletion = PendingDeviceDeletion(device)
+        deletingDevices[device.key] = pendingDeletion
+        deletingDeviceIds[device.id] = pendingDeletion
+        return pendingDeletion
+    }
+
+    private suspend fun finishPendingDeletion(pendingDeletion: PendingDeviceDeletion) {
+        withContext(NonCancellable) {
+            mutex.withLock {
+                deletingDevices.remove(pendingDeletion.device.key, pendingDeletion)
+                deletingDeviceIds.remove(pendingDeletion.device.id, pendingDeletion)
+            }
+            pendingDeletion.completed.complete(Unit)
+        }
+    }
+
     suspend fun createDevice(
         account: Account,
         deviceId: DeviceId,
@@ -126,35 +153,43 @@ class DeviceRepo @Inject constructor(
             accountId = account.id,
             path = account.path.resolve("$DEVICES_DIR/${data.id}")
         )
-        mutex.withLock {
-            if (devices[device.key] != null) throw IllegalStateException("Device already exists: ${device.key}")
-            if (devices.values.any { it.id == device.id }) {
-                throw IllegalStateException("Device ID already registered to another account: ${device.id}")
-            }
-            // Count cap is enforced under the same mutex that registers the new device,
-            // so two concurrent creates can't both pass the check.
-            val currentDeviceCount = devices.values.count { it.accountId == account.id }
-            if (currentDeviceCount >= config.maxDevicesPerAccount) {
-                throw DeviceLimitExceededException(config.maxDevicesPerAccount)
-            }
+        while (true) {
+            val pendingDeletion = mutex.withLock {
+                val pendingDeletion = deletingDevices[device.key] ?: deletingDeviceIds[device.id]
+                if (pendingDeletion != null) {
+                    pendingDeletion
+                } else {
+                    if (devices[device.key] != null) throw IllegalStateException("Device already exists: ${device.key}")
+                    if (devices.values.any { it.id == device.id }) {
+                        throw IllegalStateException("Device ID already registered to another account: ${device.id}")
+                    }
+                    // Count cap is enforced under the same mutex that registers the new device,
+                    // so two concurrent creates can't both pass the check.
+                    val currentDeviceCount = devices.values.count { it.accountId == account.id }
+                    if (currentDeviceCount >= config.maxDevicesPerAccount) {
+                        throw DeviceLimitExceededException(config.maxDevicesPerAccount)
+                    }
 
-            device.path.run {
-                if (!parent.exists()) {
-                    parent.createDirectory()
-                    log(TAG) { "Created parent dir for $this" }
-                }
-                if (!exists()) {
-                    createDirectory()
-                    log(TAG) { "Created dir for $this" }
+                    device.path.run {
+                        if (!parent.exists()) {
+                            parent.createDirectory()
+                            log(TAG) { "Created parent dir for $this" }
+                        }
+                        if (!exists()) {
+                            createDirectory()
+                            log(TAG) { "Created dir for $this" }
+                        }
+                    }
+                    device.writeDevice()
+                    lastSeenPersistedAt[device.key] = device.lastSeen
+                    log(TAG, VERBOSE) { "Device written: $this" }
+                    devices[device.key] = device
+                    log(TAG) { "createDevice(): Device created $device" }
+                    return device
                 }
             }
-            device.writeDevice()
-            lastSeenPersistedAt[device.key] = device.lastSeen
-            log(TAG, VERBOSE) { "Device written: $this" }
-            devices[device.key] = device
+            pendingDeletion.completed.await()
         }
-        log(TAG) { "createDevice(): Device created $device" }
-        return device
     }
 
     suspend fun getDevice(key: DeviceKey): Device? {
@@ -177,48 +212,77 @@ class DeviceRepo @Inject constructor(
 
     suspend fun deleteDevice(key: DeviceKey) {
         log(TAG, VERBOSE) { "deleteDevice($key)..." }
-        val toDelete = mutex.withLock {
-            devices.remove(key).also { lastSeenPersistedAt.remove(key) } ?: throw IllegalArgumentException("$key not found")
+        val pendingDeletion = mutex.withLock {
+            val toDelete = devices.remove(key).also { lastSeenPersistedAt.remove(key) }
+                ?: throw IllegalArgumentException("$key not found")
+            trackPendingDeletion(toDelete)
         }
-        toDelete.sync.withLock {
-            toDelete.path.deleteRecursively()
-            log(TAG) { "deleteDevice($key): Device deleted: $toDelete" }
+        try {
+            pendingDeletion.device.sync.withLock {
+                val toDelete = pendingDeletion.device
+                toDelete.path.deleteRecursively()
+                log(TAG) { "deleteDevice($key): Device deleted: $toDelete" }
+            }
+        } finally {
+            finishPendingDeletion(pendingDeletion)
         }
     }
 
     suspend fun deleteDevices(accountId: AccountId) {
         log(TAG, VERBOSE) { "deleteDevices($accountId)..." }
-        val toDelete = mutex.withLock {
+        val pendingDeletions = mutex.withLock {
             devices
                 .filter { it.value.accountId == accountId }
                 .map {
                     lastSeenPersistedAt.remove(it.key)
-                    devices.remove(it.key)!!
+                    trackPendingDeletion(devices.remove(it.key)!!)
                 }
         }
-        log(TAG) { "deleteDevices($accountId): Deleting ${toDelete.size} devices" }
-        toDelete.forEach { device ->
-            device.sync.withLock {
-                device.path.deleteRecursively()
-                log(TAG) { "deleteDevices($accountId): Device deleted: $device" }
+        log(TAG) { "deleteDevices($accountId): Deleting ${pendingDeletions.size} devices" }
+        var firstFailure: Throwable? = null
+        pendingDeletions.forEach { pendingDeletion ->
+            try {
+                val device = pendingDeletion.device
+                device.sync.withLock {
+                    device.path.deleteRecursively()
+                    log(TAG) { "deleteDevices($accountId): Device deleted: $device" }
+                }
+            } catch (t: Throwable) {
+                if (firstFailure == null) {
+                    firstFailure = t
+                } else {
+                    firstFailure.addSuppressed(t)
+                }
+            } finally {
+                finishPendingDeletion(pendingDeletion)
             }
         }
+        firstFailure?.let { throw it }
     }
 
     suspend fun updateDevice(key: DeviceKey, action: (Device.Data) -> Device.Data) {
-        val device = devices[key] ?: return
+        val device = mutex.withLock { devices[key] } ?: return
         device.sync.withLock {
-            val newDevice = device.copy(data = action(device.data))
-            val oldWithoutLastSeen = device.data.copy(lastSeen = newDevice.lastSeen)
+            val current = mutex.withLock {
+                devices[key]?.takeIf { it.sync === device.sync }
+            } ?: return
+            val newDevice = current.copy(data = action(current.data))
+            val oldWithoutLastSeen = current.data.copy(lastSeen = newDevice.lastSeen)
             val metadataChanged = oldWithoutLastSeen != newDevice.data
-            val lastPersisted = lastSeenPersistedAt[key] ?: device.lastSeen
+            val lastPersisted = lastSeenPersistedAt[key] ?: current.lastSeen
             val shouldPersist = metadataChanged ||
                 Duration.between(lastPersisted, newDevice.lastSeen) >= LAST_SEEN_DEBOUNCE
             if (shouldPersist) {
                 newDevice.writeDevice()
-                lastSeenPersistedAt[key] = newDevice.lastSeen
             }
-            devices[key] = newDevice
+            mutex.withLock {
+                if (devices[key]?.sync === device.sync) {
+                    devices[key] = newDevice
+                    if (shouldPersist) {
+                        lastSeenPersistedAt[key] = newDevice.lastSeen
+                    }
+                }
+            }
         }
     }