[FIX] component: protect against user code executing in critical section

Canceling a fiber may cause user code to be run, which means that some
new renderings could be scheduled, but this could interfere with the
current renderings!

src/component/component_node.ts

@@ -79,11 +79,9 @@ export function component<P extends object>(
   let node: any = ctx.children[key];
   let isDynamic = typeof name !== "string";
 
-  if (node) {
+  if (node && node.status === STATUS.DESTROYED) {
-    if (node.status === STATUS.DESTROYED) {
     node = undefined;
   }
-  }
   if (isDynamic && node && node.component.constructor !== name) {
     node = undefined;
   }

src/component/fibers.ts

@@ -16,7 +16,12 @@ export function makeRootFiber(node: ComponentNode): Fiber {
   let current = node.fiber;
   if (current) {
     let root = current.root!;
+    // lock root fiber because canceling children fibers may destroy components,
+    // which means any arbitrary code can be run in onWillDestroy, which may
+    // trigger new renderings
+    root.locked = true;
     root.setCounter(root.counter + 1 - cancelFibers(current.children));
+    root.locked = false;
     current.children = [];
     current.childrenMap = {};
     current.bdom = null;

tests/components/__snapshots__/concurrency.test.ts.snap

@@ -1289,6 +1289,46 @@ exports[`delayed rendering, then component is destroyed and  stuff 3`] = `
 }"
 `;
 
+exports[`destroyed component causes other soon to be destroyed component to rerender, weird stuff happens 1`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  
+  return function template(ctx, node, key = \\"\\") {
+    let b2,b3;
+    b2 = text(\` A \`);
+    if (ctx['state'].flag) {
+      const b4 = component(\`B\`, {value: ctx['state'].valueB}, key + \`__1\`, node, ctx);
+      const b5 = component(\`C\`, {value: ctx['state'].valueC}, key + \`__2\`, node, ctx);
+      b3 = multi([b4, b5]);
+    }
+    return multi([b2, b3]);
+  }
+}"
+`;
+
+exports[`destroyed component causes other soon to be destroyed component to rerender, weird stuff happens 2`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  
+  return function template(ctx, node, key = \\"\\") {
+    return text(ctx['props'].value);
+  }
+}"
+`;
+
+exports[`destroyed component causes other soon to be destroyed component to rerender, weird stuff happens 3`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  
+  return function template(ctx, node, key = \\"\\") {
+    return text(ctx['state'].val+ctx['props'].value);
+  }
+}"
+`;
+
 exports[`destroying/recreating a subcomponent, other scenario 1`] = `
 "function anonymous(bdom, helpers
 ) {

tests/components/concurrency.test.ts

@@ -4,6 +4,7 @@ import {
   mount,
   onMounted,
   onRendered,
+  onWillDestroy,
   onWillStart,
   onWillUnmount,
   onWillUpdateProps,
@@ -3695,6 +3696,93 @@ test("another scenario with delayed rendering", async () => {
   ]).toBeLogged();
 });
 
+test("destroyed component causes other soon to be destroyed component to rerender, weird stuff happens", async () => {
+  let def = makeDeferred();
+  let c: any = null;
+
+  class B extends Component {
+    static template = xml`<t t-esc="props.value"/>`;
+    setup() {
+      useLogLifecycle();
+      onRendered(() => {
+        def.resolve();
+      });
+      onWillDestroy(() => {
+        c.state.val++;
+        c.render();
+      });
+    }
+  }
+  class C extends Component {
+    static template = xml`<t t-esc="state.val + props.value"/>`;
+    state = useState({ val: 0 });
+    setup() {
+      c = this;
+      useLogLifecycle();
+    }
+  }
+
+  class A extends Component {
+    static template = xml`
+      A
+      <t t-if="state.flag">
+        <B value="state.valueB"/>
+        <C value="state.valueC"/>
+      </t>`;
+    static components = { B, C };
+    state = useState({ flag: false, valueB: 1, valueC: 2 });
+    setup() {
+      useLogLifecycle();
+    }
+  }
+
+  const parent = await mount(A, fixture);
+  expect(fixture.innerHTML).toBe(" A ");
+  expect(["A:setup", "A:willStart", "A:willRender", "A:rendered", "A:mounted"]).toBeLogged();
+
+  // initiate a render in A, but is blocked in B
+  parent.state.flag = true;
+
+  await def;
+  await nextMicroTick();
+  expect([
+    "A:willRender",
+    "B:setup",
+    "B:willStart",
+    "C:setup",
+    "C:willStart",
+    "A:rendered",
+    "B:willRender",
+    "B:rendered",
+    "C:willRender",
+    "C:rendered",
+  ]).toBeLogged();
+
+  // initiate render in A => will cancel renders in B/C and restarts
+  parent.state.valueB = 2;
+  await nextTick();
+  expect([
+    "B:willDestroy",
+    "C:willDestroy",
+    "A:willRender",
+    "B:setup",
+    "B:willStart",
+    "C:setup",
+    "C:willStart",
+    "A:rendered",
+    "B:willRender",
+    "B:rendered",
+    "C:willRender",
+    "C:rendered",
+    "A:willPatch",
+    "C:mounted",
+    "B:mounted",
+    "A:patched",
+  ]).toBeLogged();
+
+  expect(fixture.innerHTML).toBe(" A 22");
+});
+
 //   test.skip("components with shouldUpdate=false", async () => {
 //     const state = { p: 1, cc: 10 };