Memory leaks

[svalaskevicius]

Hi, we've been hit by a memory leak in fs2:

lazy val catsVersion = "2.6.0"
lazy val catsEffectVersion = "3.1.0"
lazy val fs2Version = "3.0.2"

Simplest example to reproduce:

package example

import cats.effect.IOApp
import cats.effect.{ExitCode, IO}
import fs2.Stream
import fs2.Pull
import scala.concurrent.duration._

object Hello extends IOApp {

  override def run(args: List[String]): IO[ExitCode] =
    Stream
      .awakeEvery[IO](2.milli)
      .map(identity _)
      .foreach(_ => IO.unit)
      .compile
      .drain
      .as(ExitCode.Success)

}

The fs2 pull/bind etc objects keep being accumulated in memory and never freed, also, they seem to form a cycle:

Also, a colleague mentioned, that Stream.repeat method also leaks, and while I haven't looked at that, it may be related.

Needless to say, this is a rather serious issue and is a blocker for us.

[diesalbla]

@svalaskevicius Good afternoon, Sarunas.

We would like to help you fix this issue as soon as possible, although of course we may need some help. For some convenience, could you please open a Pull Request adding a test, or an executable program with that leak? That would help us work on it.

Also, on which version of fs2 have you found this problem? If you could try to reproduce it in previous versions of fs2, that would help us to bisect the problem.

[svalaskevicius]

Hi! Both the fs2 version and the program are in the description, as snippets of text. That is the full program that can replicate it. I can ofc upload the whole project as well - will do as a pr, as you've suggested

[nikiforo]

@diesalbla
fs2 has MemoryLeakSpec, however, it's located at core/jvm/src/it/scala/fs2 and isn't executed when coreJVM/ testOnly fs2.MemoryLeakSpec. After I moved the spec to core/jvm/src/test/scala/fs2 I was able to execute the spec with coreJVM/ testOnly fs2.MemoryLeakSpec. So, should MemoryLeakSpec be moved to src/test?

[diesalbla]

So, should MemoryLeakSpec be moved to src/test?

I cannot recall why it was kept outside of the unit tests folder src/test, so I cannot answer that. @mpilquist may.

[svalaskevicius]

@diesalbla , please see #2395 for the example project.

I have added fork in run for convenience of running with sbt run

[diesalbla]

That is the full program that can replicate it. I can ofc upload the whole project as well - will do as a pr, as you've suggested

Well, not the whole project, but if you can clone fs2 and add an App with that snippet in a PR (a branch) that would help us moving. About the errors, the BindBind objects are introduced during stream compilation, to rotate left-associated streams ((a >>= f) >>= g) into right-associated ones (a >> (\x -> f x >>= g)), otherwise the compilation would have to recurse on the left and would stack overflow.

We have been doing some changes to the pull structure and the interpreter over the last months, so we may be look (bisect) for a version in the master mainline in which the leak did not occur.

[mpilquist]

@diesalbla I pushed a reproduction to topic/awakeEveryLeak branch. To run, switch to coreJVM project and then execute IntegrationTest / testOnly *Memory*.

• Replacing awakeEvery with Stream.constant(()) or Stream(()).repeat results in no leak
• Removing foreach(_ => IO.unit) results in no leak
• Removing map(identity _) results in no leak

[svalaskevicius]

Interestingly, the project this problem occurred does not use awake every, it instead reads via http4s stream and decodes the stream to messages..

Also, if the foreach line is done before map identity - all seems ok

[mpilquist]

Here's a simpler reproduction that doesn't use awakeEvery:

Stream.constant(()).flatMap(_ => Stream(())).map(identity).flatMap(_ => Stream.empty)

Problem appears to be related to the interleaving of flatMap/map/flatMap

[prova]

This was found when we used: textStream.through(ServerSentEvent.decoder), which is then evalTapped inside a for-comprehension.

[svalaskevicius]

the results from bisect:

e25a6e1 is the first bad commit
Date: Tue Dec 29 00:41:57 2020 +0000

Pull - compilation changes...

- WE remove the use of the `BuildR` subclass of Run, as well as the
  continuation-like `CallRun` type alias.

- We remove the use of `F.attempt` after recursive calls to `go`,
  since errors are now also processed within the

- For the ViewRunner, to avoid a stack overflow in one of the texts,
  we modify the method `out` to include a manual unrolling loop.

core/shared/src/main/scala/fs2/Pull.scala | 56 ++++++++++++-------------------
1 file changed, 22 insertions(+), 34 deletions(-)

git bisect log                                                                                                                                                      201ms  Thu 13 May 2021 15:50:25 BST
# bad: [391252d1e4cb4fbc6bca6a3ca8ca237b5466cabe] Upgrade to Scala 3.0.0-RC3
# good: [69a2778184eb2d9084c20fc1560c074231914d60] Add tag publishing
git bisect start 'v3.0.2' 'v3.0.0-M1'
# skip: [7e6e1d26d1f5fb5c0e5c68dd80d0671a15d11e48] Restore socket group resource constructors
git bisect skip 7e6e1d26d1f5fb5c0e5c68dd80d0671a15d11e48
# good: [da413051016f9dd1f206d5202409bf211a870878] Test cleanup: MergeSuite
git bisect good da413051016f9dd1f206d5202409bf211a870878
# skip: [35a37b645ee925fceef76098fcfa785dc5c791ac] Some code cleanups.
git bisect skip 35a37b645ee925fceef76098fcfa785dc5c791ac
# skip: [ad9866f931e33d982e8bc0273eb470b86cfb6e9c] Fix Memory Leak detected on GroupWithin Benchmark.
git bisect skip ad9866f931e33d982e8bc0273eb470b86cfb6e9c
# bad: [d048da96275a0e8bbde1383d584c47558862a977] add simpler example of topic use
git bisect bad d048da96275a0e8bbde1383d584c47558862a977
# good: [47e6ea681f2bd87951c085200361fc1c38a2479e] Remove the R trait ADT: just use a continuation .
git bisect good 47e6ea681f2bd87951c085200361fc1c38a2479e
# bad: [5b147cc77ee326f3960557e7402cc28eec874790] Merge branch 'main' into develop
git bisect bad 5b147cc77ee326f3960557e7402cc28eec874790
# good: [95aa5c9ffcc74b93bf5e41c72d3cbd5f78bbfe42] Scala 3.0.0-M3 support
git bisect good 95aa5c9ffcc74b93bf5e41c72d3cbd5f78bbfe42
# bad: [31d0d79c4f806bdd6f4b37c06364b84bde2b9066] Merge pull request #2201 from RaasAhsan/fromqueue
git bisect bad 31d0d79c4f806bdd6f4b37c06364b84bde2b9066
# good: [e889de573f3350790393fe0a5cf31436112a8d84] scalafmt
git bisect good e889de573f3350790393fe0a5cf31436112a8d84
# good: [b5476aa3290b034614dfb406c26972801c290ef4] Update site docs
git bisect good b5476aa3290b034614dfb406c26972801c290ef4
# bad: [61d50d0a85ad7bd7ed25f355f74bbd24da89d820] Merge pull request #2199 from diesalbla/rewrite_scope_close
git bisect bad 61d50d0a85ad7bd7ed25f355f74bbd24da89d820
# bad: [1ea8e426c5a5a3c7c24a0ea2e250c8007b6fa433] Merge pull request #2196 from diesalbla/compileGo_no_attempt_or_Buildr
git bisect bad 1ea8e426c5a5a3c7c24a0ea2e250c8007b6fa433
# bad: [e25a6e1c59ec77b4c60e97731d1b152aec1c068b] Pull - compilation changes...
git bisect bad e25a6e1c59ec77b4c60e97731d1b152aec1c068b
# first bad commit: [e25a6e1c59ec77b4c60e97731d1b152aec1c068b] Pull - compilation changes...