jwstegemann / fritz2

Easily build reactive web-apps in Kotlin based on flows and coroutines.
https://www.fritz2.dev
MIT License
653 stars 28 forks source link

Proper management of Store Lifecycle (Jobs, Handlers and Flows) #806

Closed metin-kale closed 11 months ago

metin-kale commented 12 months ago

Motivation

Currently there is no real lifecycle-management for Jobs of Handlers and Stores. The initial idea was to hide those details from the user in order to make the API and overall user-experience as easy and pleasant as possible.

Sadly this was a bad idea - not only does it lead to memory leaks, but also to unexpected behaviour due "handledBy" invocations running endlessly. Imagine a delayed change of a store's data initiated by a handler which is no longer "visible", because the Mountpoint in which the handler was called was already destroyed. This is definitely a problem, as it contradicts the reliable reactive behaviour of fritz2.

That being said, especially the latter problem arises very seldomly, which is the reason why we encountered and subsequently adressed this problem so late on the final route to 1.0 version of fritz2.

Nevertheless we have to fix it, so here we go.

Conclusion

There is a need for an explicit lifecycle-management for all fritz2 elements which are related to reactive behaviour such as:

Within a RenderContext, those elements should rely on the managed Job of their context to be finished, as the RenderContext gets dropped by any render*-function. This solves the memory leaks as well as the unexpected behaviour of a neverending local store or handledBy call.

Outside of any RenderContext or other receivers that offer some Job, the job-handling should be made explicit. Typically this applies for global stores that hold application states. Those elements are discrete and rather small in number and are intended to run for the whole lifecycle of an application, so there is no harm in creating new jobs for those cases.

Solution

RootStores

Previously, RootStores created their own jobs which were never cancelled. With this PR, every store initialization needs a job: Job parameter.

class RootStore<D>(initialData: D, **job: Job**,  override val id: String = Id.next())
fun <D> storeOf(initialData: D, **job: Job**, id: String = Id.next())

Because the jobs within a RenderContext are already managed, we also added convenience functions which directly use the RenderContext-job if a store is created with the storeOf-factory:

// `WithJob` is a base interface of `RenderContext`
fun <D> WithJob.storeOf(initialData: D, job: Job = this.job, id: String = Id.next())
//                                                 ^^^^^^^^
//                                                 the job is taken from its receiver context

Each store which is created outside of a WithJob-Scope or directly using the constructor needs a user managed Job. The user himself is responsible to take care of the job lifecycle and cancelling the job when it is not needed anymore. We suggest using the job of the surrounding RenderContext, because that one is always properly managed.

In real world applications, it is normal to have some globally defined Stores. Don't be afraid to simply create new Jobs for those without any "management" by yourself. Those stores are intended to run for as long as the application runs. So there is no need to stop or cancel them. Forever running jobs inside such global stores were a normal occurance before this PR and will remain so after this PR.

No more global handledBy

Previously, handledBy could be called everywhere. In a WithJob-context (e.g. a RenderContext), it used that job, otherwise it created a new job which was endlessly running within the application-lifecycle, which is what might have caused some unexpected side effects.

Now you can only run handledBy within

  1. WithJob-scope (a RenderContext implements WithJob)
  2. Within a RootStore-Instance

This ensures that the Job used inside of handledBy is always properly managed, which includes both cases:

The 'handledBy'-functions within a RootStore are protected and can therefore only be used within the RootStore itself or in any derived custom store-implementation. A RootStore as receiver - e.g. using extension functions or apply/run - is not sufficient!

If you explicitely want to use the store-job outside the RootStore, you have to create an extension function with the WithJob receiver and call that function within the RootStore wrapped with the new runWithJob-function.

Example:

object MyStore : RootStore<String>("", Job()){
    init {
        runWithJob{ myFunction() }
    }
}

fun WithJob.myFunction() {
    flowOf("ABC") handledBy MyStore.update
}

Alongside with this change, a handledBy-call will also be interrupted if:

  1. The store-job has been cancelled
  2. The consumer-job (job of the scope where handledBy was called) has been cancelled

Also, the store's data-Flow will be completed when the store-job has been cancelled to prevent further side effects of the store.

Improve Rendering

When using reactive rendering, e.g. using Flow<T>.render or Flow<T>.renderEach, the render-operation was previously never interrupted if a new flow-value was emitted. The render-operations where queued after each other. This behaviour has changed with this PR. When the Flow emits a new value, the current render-task will be interrupted and it will directly re-render the content with the latest value. This will improve performance.

In rare circumstances, this might also cause different behaviour. In our opinion this could only happen when the reactive rendering is abused for state handling outside of Stores, for example with mutating some var foo outside of the rendering block. Since we consider such implementations bad practise, we recommend to change these constructs.

Migration Guide

Stores

For all global Stores of an application, just add some newly created Job:

// previously
object MyApplicationStore : RootStore<AppState>(AppState(...)) {
}

// now
object MyApplicationStore : RootStore<AppState>(AppState(...), job = Job()) {
}

// previously
val storedAppState = storeOf(AppState())

// now
val storedAppState = storeOf(AppState(), job = Job())

If you encounter a compiler error due to missing job-parameter inside a RenderContext, please have a look at the last section about "Chasing Memory Leaks" where the dangers and solutions as explained in depth.

Global handledBy Calls

You simply have to move those calls inside some Store or some RenderContext - there is no standard solution which fits all situations. You have to decide for yourself where the data handling fits best.

If you have two stores and changes to one should also change the other, consider handleAndEmit as alternate approach or move the call to the dependent store (the one that holds the handler passed to handledBy).

History

The history-function without receiver has been removed. So you either have to move the history code inside some Store (which is the common and recommended approach) or you have to invoke the constructor manually:

// previously
val myHistory = history()

// now
object MyStore : RootStore<MyData>(MyData(), job = Job()) {
    val history = history()
}

// or if really needed outside a store:
val myHistory = History(0, emptyList(), Job())
Router

If you really need to call the Router-constructors manually, you have to provide a Job now:

// previously
val router = Router(StringRoute(""))

// now
val router = Router(StringRoute(""), job = Job())

Consider using the routerOf-factories, they will create a Job automatically.

Chasing Memory Leaks

Besides the previously shown dedicated proposals for fixing compiler errors, we encourage you to scan your code for potential memory-issues we simply cannot prevent by our framework:

Imagine the creation of a Store inside a RenderContext where a new Job() is created:

// previously
fun RenderContext.renderSomething() = div {
    object MyApplicationStore : RootStore<AppState>(AppState(...)) {
    }
}

// now - satisfies compiler but is wrong!
fun RenderContext.renderSomething() = div {
    object MyApplicationStore : RootStore<AppState>(AppState(...), job = Job()) {
    //                                                             ^^^^^^^^^^^
    //                                                             This fixes the compiler error, but in most cases
    //                                                             this is *wrong*. The job-object is never cancelled
    //                                                             -> memory leak!
    }
}

// now - recommended approach
fun RenderContext.renderSomething() = div {
    object MyApplicationStore : RootStore<AppState>(AppState(...), job = this@renderSomething.job) {
    //                                                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    //                                                             Reuse job of the surrounding RenderContext
    //                                                             and you will be fine.
    }
}

Last but not least: If you really want such a store to live forever, then refactor the code and define it as a global value.

There are variations of the pattern above which are even worse:

If you encounter any job-creation - thus manually created new Job()s - inside a looping-construct, this is very likely a mistake. Typical looping constructs appears inside renderEach or simple for-loops inside a render. Inside such loops, you should also strive to re-use the Job of the surrounding RenderContext.

For example, you should absolutely change code section like this one:

val storedCustomers = storeOf<List<Customer>>(emptyList(), job = Job())

fun RenderContext.renderCustomers() = section {
    storedCustomers.renderEach { customer ->
        val editStore = object : RootStore<Customer>(customer, job = Job()) {
        //                                                     ^^^^^^^^^^^
        //                                                     This is evil! A new job is created on every customer 
        //                                                     and every re-rendering from the outer store.
        //                                                     The jobs are never canceled -> memory leak!
        }
        // render one customer somehow
    }
}

You should change such a spot by reusing the RenderContext's job:

fun RenderContext.renderCustomers() = section {
    storedCustomers.renderEach { customer ->
        val editStore = object : RootStore<Customer>(customer, job = this@renderCustomers.job) {
        //                                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        //                                                     This is good! The `job` is managed by `renderEach`
        //                                                     so it is cancelled on re-rendering and the objects
        //                                                     can be deleted by the garbage collector.
        }
        // render one customer somehow
    }
}

Keep in mind that the first shown example may reside inside a loop, from where the renderSomething-function gets called: in that case, there is also a leak inside a loop, which is even harder to encounter - so always consider the job-creation inside a RenderContext as code smell and bad practise.

Lysander commented 11 months ago

This PR has the following open TODOs imho:

Lysander commented 11 months ago

fixes #704