No rendering after drop

[Procrat]

Description

There were various memory leaks which caused the scene to stay around which tries to render after the GUI has been dropped.

There were 1500+ leaked objects. The first few commits by @Procrat tackle the actual leaks. The last commit of him tries to reduce the cloning of Application across the codebase, so debugging leaking references will be easier. @farmaazon then added fixes for last leaks and cleaned up the code.

Additionally, the message about visualization failing to attach is displayed in pop-up instead of status bar, so it will disappear after some time. Fixes #6976

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

• The documentation has been updated, if necessary.
• [ ] Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
• All code follows the
  Scala,
  Java,
  and
  Rust
  style guides. In case you are using a language not listed above, follow the Rust style guide.
• All code has been tested:
  • Unit tests have been written where possible.
  • If GUI codebase was changed, the GUI was tested when built using ./run ide build.

[wdanilo]

These changes look OK, but I'm missing info how this PR really works. Even if we are cloning world and scene in different places, all of these objects should get dropped if we drop the main app instance. Even if FRP closures keep reference to them, these networks should be dropped, so how this PR really fixes the mem leaks problems and more importantly, what is the guideline for future code to not create leaks? Also, are we able to create automated test for it?

[wdanilo]

dot missing

[wdanilo]

dot missing

[wdanilo]

dot missing

[wdanilo]

missing dot

[farmaazon]

These changes look OK, but I'm missing info how this PR really works. Even if we are cloning world and scene in different places, all of these objects should get dropped if we drop the main app instance. Even if FRP closures keep reference to them, these networks should be dropped, so how this PR really fixes the mem leaks problems and more importantly, what is the guideline for future code to not create leaks? Also, are we able to create automated test for it?

There were two causes I'm aware of

1. ApplicationData FPR network was taking ApplicationData itself, making a reference cycle.
2. Our garbage collector: it often took FRP and model of various components, possibly containing reference to Application, or World. Of course, it should be removed a couple of frames later, but it's hard to say what exactly was dropped and what not (maybe handlers for GC were?).

Removing clones was rather for reducing noise when debugging (the reports said about 1500+ references to App), and I think removing reference which is not needed is a good idea anyway.

How to mitigate it? Here we have another example of FRP network taking itself. Almost all leaks I encounter in our codebase are like that. I think the FRP could be changed a bit, to make clear that FRP should own the model (maybe special "model" behavior node, which gives mutable references to Model?

[MichaelMauderer]

QA 🟡

This looks mostly good, there are no errors after dropping the app. But when I press tab with focus in the empty window, I see the following error:

8pkg.js:3934 Uncaught Error: closure invoked after being dropped
    at imports.wbg.__wbindgen_throw (eval at <anonymous> (index.ts:296:37), <anonymous>:18716:11)
    at wasm_bindgen::throw_str::hd5a1e55c1ad9682d (pkg-opt.wasm:0x2179bdc)
    at <dyn core::ops::function::Fn<(A,)>+Output = R as wasm_bindgen::closure::WasmClosure>::describe::invoke::hde034fa7641b5b81 (pkg-opt.wasm:0x216c679)
    at __wbg_adapter_80 (eval at <anonymous> (index.ts:296:37), <anonymous>:16975:8)
    at real (eval at <anonymous> (index.ts:296:37), <anonymous>:16936:14)

Could this indicate there is still some Enso event handling active? @farmaazon

[farmaazon]

Could this indicate there is still some Enso event handling active? @farmaazon

Both yes and no: it means that the closure was dropped in rust successfully, but JS still had reference to it. See https://rustwasm.github.io/wasm-bindgen/api/wasm_bindgen/closure/struct.Closure.html

Tracking the offending closure may be tricky, but I'll try to find it.

Closure in wasm_bindgen::closure - Rust

A handle to both a closure in Rust as well as JS closure which will invoke the Rust closure.

[farmaazon]

Added more serious tracking of Scene and SceneData, and it turns out there still are many leaks:

1. init_webgl_2_context is called with Scene as an argument, which is captured to lambdas, whose handlers are in turn kept inside Scene.
2. CachedShapesPass keeps scene while being also kept inside Renderer.
3. font::Registry::init_and_load_embedded_fonts clones scene and put it into structure returned as an extension - and the extensions are also kept inside scene
4. Finally, the scene is stored in SCENE global variable but never removed from there.

I still don't know why only keydown handlers raises an error, but those Scene leaks are serious.

[farmaazon]

Because the problems are not actually regressions (there were before this PR) and this PR improves things, I will merge it, but won't close the task #6505.