arrow-kt / arrow

Λrrow - Functional companion to Kotlin's Standard Library
http://arrow-kt.io
Other
6.18k stars 452 forks source link

autoCloseScope: the order of close method calls is not the same as `AutoCloseable.use` #3386

Closed hoc081098 closed 8 months ago

hoc081098 commented 8 months ago

What version are you currently using?

1.2.3

What would you like to see?

Preproduce by https://github.com/hoc081098/kotlin_playground/blob/master/src/main/kotlin/com/hoc081098/kotlin_playground/arrowkt/autoCloseable.kt

class FirstAC : AutoCloseable {
  init {
    println("$this is created")
  }
  override fun close() = println("$this is closed")
  override fun toString() = "[1]"
}

class SecondAC(private val firstAC: FirstAC) : AutoCloseable {
  init {
    println("$this is created")
  }
  override fun close() = println("$this is closed")
  override fun toString() = "[2]"
}

class ThirdAC(private val secondAC: SecondAC, private val firstAC: FirstAC) : AutoCloseable {
  init {
    println("$this is created")
  }

  suspend fun doSomething(): Int {
    println("$this do something")
    delay(1000)
    return Random.nextInt()
  }

  override fun close() = println("$this is closed")
  override fun toString() = "[3]"
}

@OptIn(ExperimentalStdlibApi::class)
fun main(): Unit = runBlocking {
  val r1 = FirstAC().use { f ->
    SecondAC(f).use { s ->
      ThirdAC(s, f).use { t ->
        t.doSomething()
      }
    }
  }

  println("-".repeat(80))

  val r2 = autoCloseScope {
    val first = install(FirstAC())
    val second = install(SecondAC(first))
    val third = install(ThirdAC(second, first))
    third.doSomething()
  }
}

The console output:

[1] is created
[2] is created
[3] is created
[3] do something
[3] is closed
[2] is closed
[1] is closed
--------------------------------------------------------------------------------
[1] is created
[2] is created
[3] is created
[3] do something
[1] is closed
[2] is closed
[3] is closed

The order of close method calls

AutoCloseable.use: 3 -> 2 -> 1 autoCloseScope: 1 -> 2 -> 3

nomisRev commented 8 months ago

Oh, that's indeed incorrect. It should be the same as AutoCloseable.use of course. Silly, not sure how I missed that 🤔

Thanks for creating the report @hoc081098! I think we need to get another patch out in a couple days, before we switch to 2.x.x 😞

hoc081098 commented 8 months ago

@nomisRev I have created PR #3387, which should fix this issue 🙏