lihaoyi/scala.rx

How to handle `Ctx.Owner` in classes

Semptic opened this issue ยท 19 comments

I try to avoid adding (implicit ctx: Ctx.Owner) to each class with following approach:

class Test {
    implicit val ctx: Ctx.Owner = Ctx.Owner.safe()

    var count = 0
    val a = Var(1); val b = Var(2)
    def mkRx(i: Int)(implicit ctx: Ctx.Owner) = Rx { count += 1; i + b() }
    val c = Rx {
      val newRx = mkRx(a())
      newRx()
    }
    println(c.now, count)
    a() = 4
    println(c.now, count)
    b() = 3
    println(c.now, count) //(7,5) -- 5??

    (0 to 100).foreach { i => a() = i }
    println(c.now, count)
    b() = 4
    println(c.now, count) //(104,211) -- 211!!!
  }

This seams to work but I don't know if this is really correct or if there is a better way to do this.

The documentation seems to be a bit lacking on information how to structure more complex real life apps.
I'm struggling with similar issue as @Semptic . I'm using scala.rx with scalajs-react components. I'd like to be able to use Rx(...) and create observers freely within that component and be able to destroy them all when the component is destroyed (unmounted) to avoid memory leaks and undesired side-effects (for observers).

I'd like to be able to do something like this:

class ComponentBackend {
    implicit val ctx: Ctx.Owner = Ctx.Owner.safe()

    // use Rx(...) and create Obs

    def onUnmount() = {
        ctx.kill() // Kill all rxs and obs in one go but there's no such method
    }
}

I think I understand and I might have an idea - basically you want a managed context that can be freed all at once. I can poke around for a bit and see what I come up with.

A comment in the docs about whether this is a sound thing to do would be useful?

@Voltir Did you check out the managed context owner thing? It seems like it could be useful for scalajs when you have components that might not need state if they aren't being viewed?

migo commented

I'd also like to know how to handle the owner context correctly. More documentation would be useful.
Especially:

  • How do you set up an initial Rx when you do not have an implicit Ctx.Owner available yet? Do you always have to create an implicit Ctx.Owner.safe() before that? What about Rx.unsafe? Is this an option for an initial Rx? It doesn't seem so, because I also get No implicit Ctx.Owner is available here! when using Rx.unsafe without an implicit owner.

Just upgraded from Scala.Rx 0.2.8, and I've also been struggling with this one. I agree with @migo that the real question is, how should you create an ownership context at your "top" level? Is @msatala's solution (to put Ctx.Owner.safe() implicitly into the class, and then .kill it when done) reasonably correct?

Even if we don't get a built-in solution, I'd like to understand if this is a reasonable approach...

Answering my own question: no @msatala's solution doesn't work. Is there any plausible way to tame this currently, even a workaround that lets you trace manually down from the root Ctx.Owner to its dependencies?

migo commented

@jducoeur I now just create one implicit val ctx: Ctx.Owner = Ctx.Owner.safe() on the top level of my app (e.g. in the JSApp class) and pass this context around everywhere. This seems to work, re-evaluated Rx values are cleaned up correctly (as far as I can see).

@migo The question (which I really have no idea about) is whether, when the Ctx.Owner goes out of scope, will everything under it also get cleaned up? If so, then I think I'm good: I'm generating the Ctx.Owner as part of building each page, and attaching the Rx'es to that. But since there is no explicit termination, I have no clue whether everything will clean up naturally as part of garbage collection when that page goes away, or whether more is needed.

(Normally I'd just go into the source code and answer for myself, but the library is so macro-centric that I haven't yet puzzled out how everything works...)

migo commented

@jducoeur Sorry, I don't know about that either.
But if you pull the context one step up (into the JSApp class), so that you can pass it into all your pages, it would definitely be fine.

True, but that's not actually the concern. At this point, I'm successfully defining the Ctx.Owner in Page and passing it down as needed. What I don't yet know is whether this might be leaking. Having the Owner defined at the JSApp level will certainly leak -- it means that stuff doesn't get released until the end of the Application, which is much longer than I'd like. (Since you might be in the JSApp for many, many Pages -- hours, even weeks.)

I would like to release all of the Rx'es when we change Pages (since that's the obvious time to do so), and that might already be happening automatically -- that's when the Ctx.Owner itself goes out-of-scope and ought to get garbage-collected. But not yet knowing the internals, I don't know whether this works out of the box or not. If the Owner going out of scope does not transitively cause the owned Rx'es to also get garbage-collected, then I'm probably slowly leaking them. I'm hoping somebody who knows the internals can venture an opinion.

(Eventually I'll get myself read into the code, but lacking a clear design document, it's more challenging than average -- it's not like Scalatags, where the code is pretty straightforward and well-documented...)

From what I understand, an App-level Owner is perfectly fine. You can define it with the Ctx.Owner.safe() macro, which will create a Ctx.Owner.Unsafe. It additionally ensures that you are not within another Owner.Unsafe. All subscriptions that are done with this app level owner in scope will stay forever and never be cleaned up, which is fine as this is supposed to be the global state. Now, it gets interesting when you are actually nesting operations in Rx.

All mapping-operations like map/flatMap/filter/Rx {...} on Rx are macros that generate code to assure that all subscriptions in the expression-body of the operation are cleaned up. It creates a new owner (not Unsafe) for each trigger in the operation and injects this new owner into the expression-body. Then subscriptions using this owner implicitly will be cleaned up when a dependency of this new owner trigger a new value. This is done via the update mechanism of scala.rx.

So it is really important to always have an implicit Ctx.Owner (or Ctx.Data but that is another topic) on your nested functions. If you use a function in an Rx operation you need this implicit owner on your function, so the macro can inject the new owner into the call to this function.

Starting with the App-level Owner, all operations on this level will run forever and make your app run. If you now nest reactive components by mapping over an Rx, you just pass the owner and they will only live for the lifetime of its parent.

@jducoeur I stumbled across the same exact problem as you did. Multiple pages in a Scala.js application where I am planning to give each page an Owner and let it be garbage collected once the user navigates to another page. Did you ever find out if this works without explicitly calling something like ctx.kill()?

Currently I am using a top-level application owner, but that leads to a lot of leaking of detached DOM elements (using scalatags).

Edit: I tried formulating this question on Stackoverflow: https://stackoverflow.com/questions/57330056/garbage-collection-of-ctx-owner-in-scala-rx

@fbaierl Honestly don't remember -- it's been a couple of years since my head was in this code. I haven't had any problems with this in production, but that doesn't prove anything: we could be slowly leaking and just haven't stress-tested it hard enough. It's still an open issue as far as I'm concerned...

@fbaierl Is there a specific reason you want to have an owner per Page? The most simple way of not leaking with rx, is to really pass the implicit owner through all of your methods and just have one single Owner.Unsafe in your app at top-level. With this approach you nest your owners and outdated owners are automatically killed by rx.

Take the following example:

implicit val owner = Ctx.Owner.safe() // ensures you are in a static context and returns an `Owner.Unsafe` or an implicit owner
val firstRx: Rx[Int] = ???
def doSomething(i: Int)(implicit owner: Ctx.Owner) = ???
val secondRx = Rx {  doSomething(firstRx()) } // this Rx gets the owner implicitly and will be run forever, but the Rx also creates a new owner whenever it triggers. It passes this new owner downstream and kills the old owner from the previous run.

An Owner.Unsafe (which you get from safe()) does not track subscriptions, so they can never be killed (except manual for each subscription). For other owners, you can kill all subscriptions via owner.contextualRx.kill(). I needed to create some custom owners in my app and do it this way:

def createManualOwner(): Ctx.Owner = new Ctx.Owner(new Rx.Dynamic[Unit]((_,_) => (), None))

You can safely call owner.contextualRx.kill() on this one, because it is a normal owner that tracks subscriptions (opposed to Owner.Unsafe).

Thanks for your input @cornerman. I might be totally misunderstanding something but here are my thoughts on that:

The problem I'd imagine there to be is that as I rebuild the web-page (by navigating to another page), the previously bound Rxs will still be referenced by this one top-level owner that I gave them (even if the bound Rxs never trigger again). Since the owner is never garbage collected, the bound Rxs won't be either. Now, using scalatags and scalatags-rx, I am binding DOM elements to these Rxs, which will lead to tons of detached DOM elements (since they are not garbage collected) and slow down my application over time.

If I understand your answer correctly, however, I could do what I want to do using your method of creating these page-level owners like this:

def createManualOwner(): Ctx.Owner = new Ctx.Owner(new Rx.Dynamic[Unit]((_,_) => (), None))

Just to clarify - if I create Rxs using this owner like that:

implicit val pageOwner = new Ctx.Owner(new Rx.Dynamic[Unit]((_,_) => (), None)) // val since I only want to have one per page
val firstRx: Rx[Int] = ???
// ...
// user navigates away from page
def onExit = {
  pageOwner.contextualRx.kill()
  // did this kill firstRx? (*1)
}

does that ensure that my firstRx is correctly 'killed' and can be garbage collected (see (*1))?

The problem I'd imagine there to be is that as I rebuild the web-page (by navigating to another page), the previously bound Rxs will still be referenced by this one top-level owner that I gave them (even if the bound Rxs never trigger again).

I have used scalatags before and now use outwatch. Both are quite similar with regards how observables are interpreted and normally the nesting of owners should just work as you want. Let's take a look how a page rendering could be implemented:

def welcomePage(implicit owner: Ctx.Owner) = div("Welcome")
def contentPage(implicit owner: Ctx.Owner) = div("Content", myContent...)

implicit val owner = Ctx.Owner.safe()

val pageRx = Var(Page.Welcome)

val pageComponent = pageRx map { // read this as: pageRx.map { implicit owner =>
  case Page.Welcome => welcomePage
  case Page.Content => contentPage
}

div(
  pageComponent
)

Whenever you now write a new value into pageRx (pageRx() = Page.Content), then the owner that is passed to the method pageWelcome in the previous run will be killed (including removing all downstream observers). So any subscription happening in pageWelcome or pageContent will be cleaned up for, whenever a new page is written to the rx. So, if you construct it this way, all the cleanup should be happening automatically for you. The top-level owner does not know about any subscription in these methods, only the owner that is created in each trigger of rx does know these (so there are no references from the top-level owner to the nested owner). Check out how an rx updates (https://github.com/lihaoyi/scala.rx/blob/master/scalarx/shared/src/main/scala/rx/Rx.scala#L194), it clear ownerships and creates a new Owner.

does that ensure that my firstRx is correctly 'killed' and can be garbage collected (see (*1))?

Yep, it will definitely kill the subscription and remove the observer from firstRx. Whether firstRx can be garbage-collected depends on whether it is still referenced somewhere.

So, if you construct it this way, all the cleanup should be happening automatically for you.

That is awesome. Thank you very much. My architecture was completely different and I guess that is why the cleanup did not work for me, but I will try doing it as you suggested (since doing it your way looks much cleaner anyway). Before I was just using import rx.Ctx.Owner.Unsafe._ wherever I needed to use an Rx.

Thanks a lot for that.

In the end I rewrote our application to only have one single implicit val owner = Ctx.Owner.safe() like @cornerman describes in his post and pass that around everywhere I need it. That was the easiest way and after a few weeks of testing, I am pretty confident it solved our memory leak issues. Thanks again for the help.