From 484c0a246a39fd1a62022ce5102472eca0f65bc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petrus=20Nguy=E1=BB=85n=20Th=C3=A1i=20H=E1=BB=8Dc?= Date: Tue, 5 Mar 2024 20:25:32 +0700 Subject: [PATCH] fix(autoclose): close `Closeable`s in reversed order (#3387) * fix(autoclose): close closeables in reversed order issue https://github.com/arrow-kt/arrow/issues/3386 * fix(autoclose): close closeables in reversed order issue https://github.com/arrow-kt/arrow/issues/3386 * fix test: suppressedExceptions --- .../commonMain/kotlin/arrow/AutoCloseScope.kt | 2 +- .../commonTest/kotlin/arrow/AutoCloseTest.kt | 40 ++++++++++++++++++- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/arrow-libs/core/arrow-autoclose/src/commonMain/kotlin/arrow/AutoCloseScope.kt b/arrow-libs/core/arrow-autoclose/src/commonMain/kotlin/arrow/AutoCloseScope.kt index 219d853c716..d42f23cd54d 100644 --- a/arrow-libs/core/arrow-autoclose/src/commonMain/kotlin/arrow/AutoCloseScope.kt +++ b/arrow-libs/core/arrow-autoclose/src/commonMain/kotlin/arrow/AutoCloseScope.kt @@ -98,7 +98,7 @@ internal class DefaultAutoCloseScope : AutoCloseScope { } fun close(error: Throwable?): Nothing? { - return finalizers.get().fold(error) { acc, function -> + return finalizers.get().asReversed().fold(error) { acc, function -> acc.add(runCatching { function.invoke(error) }.exceptionOrNull()) }?.let { throw it } } diff --git a/arrow-libs/core/arrow-autoclose/src/commonTest/kotlin/arrow/AutoCloseTest.kt b/arrow-libs/core/arrow-autoclose/src/commonTest/kotlin/arrow/AutoCloseTest.kt index 85ccd7966e2..2dc8d63d192 100644 --- a/arrow-libs/core/arrow-autoclose/src/commonTest/kotlin/arrow/AutoCloseTest.kt +++ b/arrow-libs/core/arrow-autoclose/src/commonTest/kotlin/arrow/AutoCloseTest.kt @@ -4,6 +4,8 @@ import arrow.atomic.AtomicBoolean import io.kotest.assertions.throwables.shouldThrow import io.kotest.matchers.shouldBe import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.channels.Channel +import kotlinx.coroutines.channels.toList import kotlinx.coroutines.test.runTest import kotlin.coroutines.cancellation.CancellationException import kotlin.test.Test @@ -76,7 +78,7 @@ class AutoCloseTest { } e shouldBe error - e.suppressedExceptions shouldBe listOf(error2, error3) + e.suppressedExceptions shouldBe listOf(error3, error2) promise.await() shouldBe error wasActive.await() shouldBe true res.isActive() shouldBe false @@ -134,6 +136,42 @@ class AutoCloseTest { res.isActive() shouldBe false } + @Test + fun closeInReversedOrder() = runTest { + val res1 = Resource() + val res2 = Resource() + val res3 = Resource() + + val wasActive = Channel(Channel.UNLIMITED) + val closed = Channel(Channel.UNLIMITED) + + autoCloseScope { + val r1 = autoClose({ res1 }) { r, _ -> + closed.trySend(r).getOrThrow() + r.shutdown() + } + val r2 = autoClose({ res2 }) { r, _ -> + closed.trySend(r).getOrThrow() + r.shutdown() + } + val r3 = autoClose({ res3 }) { r, _ -> + closed.trySend(r).getOrThrow() + r.shutdown() + } + + wasActive.trySend(r1.isActive()).getOrThrow() + wasActive.trySend(r2.isActive()).getOrThrow() + wasActive.trySend(r3.isActive()).getOrThrow() + wasActive.close() + } + + wasActive.toList() shouldBe listOf(true, true, true) + closed.receive() shouldBe res3 + closed.receive() shouldBe res2 + closed.receive() shouldBe res1 + closed.cancel() + } + @OptIn(ExperimentalStdlibApi::class) private class Resource : AutoCloseable { private val isActive = AtomicBoolean(true)