typelevel / fs2

Compositional, streaming I/O library for Scala
https://fs2.io
Other
2.38k stars 603 forks source link

Fix resource leak in `Scope`. #3473

Closed ValdemarGr closed 2 months ago

ValdemarGr commented 2 months ago

I minimized a bug I had to this.

IO.ref(0).flatMap { ref =>
  val res = Stream.resource(Resource.make(ref.update(_ + 1))(_ => ref.update(_ - 1)))

  (0 to 100000).toList.parTraverse_ { _ =>
    val fa = res.evalMap(_ => IO.sleep(10.millis)).take(10).compile.drain
    IO.race(fa, IO.sleep(90.millis))
  } >> ref.get.map(assertEquals(_, 0))
}

The Ref will sometimes not contain 0. I tried the same test with the following fix and it does not occur anymore.

Explanation (or rather my hypothesis)

If cancellation of the stream occurs between state.modify and the subsequent flatMap then the scope's state is Closed, thus when the root scope is cancelled and traverses the scope tree, the node is now Closed, but the finalizers for the attached resources were never run.

I can't seem to come up with a good test for this.

mpilquist commented 2 months ago

Thanks! Fix looks good. Would really like a test case for this though. Maybe something based on your minimization?

ValdemarGr commented 2 months ago

I've added a test.

I tried to use a seeded TestControl and make it property based, but it was still flaky and much slower.