Skip to content

Commit

Permalink
merge: Fix the lock bug
Browse files Browse the repository at this point in the history
Closes #101

See merge request opensavvy/pedestal!91
  • Loading branch information
maximegirardet committed Sep 10, 2023
2 parents d3d5920 + 4ad3d11 commit 4fdb35d
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 32 deletions.
45 changes: 43 additions & 2 deletions .gitlab-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,34 @@ workflow:
tags:
- saas-macos-medium-m1

# region Environment and dependencies

chromium:build:
extends: [ .os.docker.build ]
needs: [ ]
stage: docker

variables:
dockerfile: .gitlab/chromium.dockerfile
context: .gitlab
image: chromium

interruptible: true

chromium:publish:
extends: [ .os.docker.rename ]
needs: [ chromium:build ]
stage: deploy

variables:
image: chromium

rules:
- if: $CI_COMMIT_REF_NAME == $CI_DEFAULT_BRANCH

interruptible: true

# endregion
# region Check

check-jvm:
Expand All @@ -33,7 +61,7 @@ check-jvm:

script:
- >
./gradlew koverMergedHtmlReport koverVerify
./gradlew koverMergedHtmlReport koverVerify --continue
-PappVersion=$project_version
- mv build/reports/kover/merged/html coverage

Expand All @@ -54,11 +82,24 @@ check-ios:

script:
- >
./gradlew iosSimulatorArm64Test
./gradlew iosSimulatorArm64Test --continue
-PappVersion=$project_version
interruptible: true

check-js:
extends: [ .os.gradle ]
image: $CI_REGISTRY_IMAGE/chromium:build-$CI_PIPELINE_IID
stage: test
needs: [ chromium:build, os.version ]

script:
- >
./gradlew jsBrowserTest --continue
-PappVersion=$project_version
interruptible: true

# endregion
# region Documentation

Expand Down
7 changes: 7 additions & 0 deletions .gitlab/chromium.dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
FROM node:lts

RUN apt-get update && \
apt-get install -y --no-install-recommends default-jdk chromium

ENV CHROME_BIN=chromium
ENV CHROMIUM_FLAGS="--no-sandbox"
24 changes: 0 additions & 24 deletions spine/src/commonTest/kotlin/ServiceTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ package opensavvy.spine
import kotlinx.coroutines.ExperimentalCoroutinesApi
import opensavvy.backbone.Ref
import opensavvy.spine.Route.Companion.div
import kotlin.test.Test
import kotlin.test.assertEquals

/*
* Example API:
Expand Down Expand Up @@ -69,25 +67,3 @@ private class Api : Service("v2") {
val departments = Departments()
val users = Users()
}

class ServiceTest {

@Test
fun routeGeneration() {
val api = Api()

val routes = api.routesRecursively.joinToString(separator = "\n")

val expected = """
v2/departments
v2/departments/{department}
v2/departments/{department}/users
v2/users
v2/users/{user}
v2/users/{user}/departments
""".trimIndent()

assertEquals(expected, routes)
}

}
23 changes: 17 additions & 6 deletions state/src/commonTest/kotlin/FailureEndToEndTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import arrow.core.raise.ensure
import arrow.core.raise.ensureNotNull
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock
import kotlinx.coroutines.test.runTest
import opensavvy.state.arrow.out
import opensavvy.state.outcome.Outcome
Expand Down Expand Up @@ -52,7 +51,7 @@ class FailureEndToEndTest {

val newId = Id(Random.nextInt())

lock.withLock("create() by ${context.user}") {
lock.withLockHack("create() by ${context.user}") {
data[newId] = Counter(context.user, 0, emptySet())
}

Expand All @@ -62,7 +61,7 @@ class FailureEndToEndTest {
suspend fun list(context: Context) = out<Failures.List, List<Id>> {
ensureNotNull(context.user) { Failures.Unauthenticated }

lock.withLock("list() by ${context.user}") {
lock.withLockHack("list() by ${context.user}") {
val user: User = context.user

data
Expand All @@ -76,7 +75,7 @@ class FailureEndToEndTest {
suspend fun get(context: Context, id: Id) = out {
ensureNotNull(context.user) { Failures.Unauthenticated }

val counter = lock.withLock("get($id) by ${context.user}") { data[id] }
val counter = lock.withLockHack("get($id) by ${context.user}") { data[id] }
ensureNotNull(counter) { Failures.NotFound(id) }
ensure(counter.readableBy(context.user)) { Failures.NotFound(id) } // Do not tell the user why they cannot see it

Expand All @@ -89,7 +88,7 @@ class FailureEndToEndTest {
ensure(context.user == counter.owner) { Failures.NotTheOwner(id) }

// Possible data race here, but it's an imaginary example, so it's not a big deal
lock.withLock("increment($id) by ${context.user}") {
lock.withLockHack("increment($id) by ${context.user}") {
data[id] = counter.copy(value = counter.value + 1)
}
}
Expand All @@ -102,7 +101,7 @@ class FailureEndToEndTest {
ensure(context.user == counter.owner) { Failures.NotTheOwner(id) }

// Possible data race here, but it's an imaginary example, so it's not a big deal
lock.withLock("share($id, $user) by ${context.user}") {
lock.withLockHack("share($id, $user) by ${context.user}") {
data[id] = counter.copy(canRead = counter.canRead + user)
}
}
Expand Down Expand Up @@ -355,3 +354,15 @@ class FailureEndToEndTest {
)
}
}

private suspend inline fun <T> Mutex.withLockHack(owner: Any? = null, block: () -> T): T {
lock(owner)
val result = try {
block()
} catch (e: Throwable) {
unlock(owner)
throw e
}
unlock(owner)
return result
}

0 comments on commit 4fdb35d

Please sign in to comment.