[crash] slot content re-renders when it should be destroyed

[seb-odoo]

I'm unfortunately not able to reproduce with simple playground example.
The real use case involves either/and multiple reactive and unlucky timings.

But I want to raise awareness of the issue in case it wasn't already known. Maybe a scenario and/or solution can be theory crafted based on the code?

The PR were the issue is detected:

odoo/odoo#156066

Unfortunately it's not easily reproducible in Odoo either, even with the steps provided in that PR some people have the issue (always?), others not (never?).

What is happening there:

Adding this t-if on the slot content (the div) fixes the issue, moving up the t-if on the slot container (ActionSwiper) does not fix the issue.

This is what led me to the conclusion that even with an explicit condition to not render the whole component, the slot content still gets rendered first, which can cause a crash.

[sdegueldre]

Hm. Because rendering is async, it's possible that the condition on the component could have been true when Owl evaluated the condition on the component, and that the condition has become false before Owl gets to render the slot. In the typical case, when there are no willStart hooks registered, there is one microtask of delay between the creation of a component and its rendering.

Here is a simple example reproducing the issue on the playground

Now obviously nobody would do this on purpose but you can see how it might happen by accident under specific circumstances.

Long term, I'd like for Owl to give stronger guarantees with regards to rendering synchronicity but as it stands, Owl only guarantees "semi-synchronous" rendering for components that don't use async hooks (willStart/willUpdateProps). Semi-synchronous in this context meaning that the rendering will be finished before the next macrotask in the queue is processed. Unfortunately Owl also doesn't give any guarantees that the render will start in the next macrotask so it's not particularly difficult to schedule a render and then having promise chains invalidate the state that is currently being rendered.

[seb-odoo]

Thank you for the investigation, the example is even more simple than I imagined!

Are you implying it is not going to be fixed for now?
And if so, shouldn't we defensively re-add conditions on every component? Only inside slots?

[sdegueldre]

@ged-odoo what do you think? I think I still have that one branch somewhere that makes rendering fully synchronous when possible and it may solve this problem.

[ged-odoo]

this is really interesting, and complicated. the issue is much larger than slots: basically, since there is a period of time between component is instantiated and when it is rendered, there also may have changes that cause the component to be invalid.

making owl semi-synchronous would reduce the surface area of that problem, but it is not a complete solution. (note that i am not against the change, just afraid of it)

[ged-odoo]

not sure what to do, but we should talk about it :)