NextERP/owl
2
0
Fork 0
mirror of https://github.com/odoo/owl.git synced 2025-10-06 19:59:41 +07:00
Code Issues Packages Projects Releases Wiki Activity

[FIX] component: resolve a subtle concurrency issue

Browse Source 
Asynchronous rendering is subtle. Before this commit, it was possible
(though not easy) to get into a situation where the Owl rendering
pipeline decremented twice the counter of a fiber that indicates that
its work is complete.

The situation occurs in the `render` method, where we create a fiber,
wait for a micro tick before starting the actual rendering (the goal was
to batch all changes coming from a single call stack). However, in this
microtask tick, it was possible that the fiber was cancelled, and we did
not have a check for that.
This commit is contained in:
Géry Debongnie
2020-02-06 22:35:41 +01:00
committed by aab-odoo
parent 3fdc7a48f3
commit ee956a1977
2 changed files with 217 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/component/component.ts
+3
View File
@@ -373,6 +373,9 @@ export class Component<Props extends {} = any, T extends Env = Env> {
const fiber = new Fiber(null, this, force, null);
Promise.resolve().then(() => {
if (__owl__.isMounted || !isMounted) {
if (fiber.isCompleted) {
return;
}
// we are mounted (__owl__.isMounted), or if we are currently being
// mounted (!isMounted), so we call __render
this.__render(fiber);
tests/component/async.test.ts
+214
View File
@@ -1152,6 +1152,220 @@ describe("async rendering", () => {
expect(fixture.innerHTML).toBe("<div><span>0</span><span>1</span></div>");
});
test("concurrent renderings scenario 14", async () => {
let b: B | undefined = undefined;
let c: C | undefined = undefined;
class C extends Component {
static template = xml`
<p>
<span t-esc="props.fromA"/>
<span t-esc="props.fromB"/>
<span t-esc="state.fromC"/>
</p>`;
state = useState({ fromC: 3 });
constructor(parent, props) {
super(parent, props);
c = this;
}
}
class B extends Component {
static template = xml`<p><C fromB="state.fromB" fromA="props.fromA"/></p>`;
static components = { C };
constructor(parent, props) {
super(parent, props);
b = this;
}
state = useState({ fromB: 2 });
}
class A extends Component {
static template = xml`<p><B fromA="state.fromA"/></p>`;
static components = { B };
state = useState({ fromA: 1 });
}
const a = new A();
await a.mount(fixture);
expect(fixture.innerHTML).toBe(
"<p><p><p><span>1</span><span>2</span><span>3</span></p></p></p>"
);
// trigger a re-rendering of the whole tree
a.state.fromA += 10;
// wait enough for the whole tree to be re-rendered, but not patched yet
await nextMicroTick();
await nextMicroTick();
await nextMicroTick();
await nextMicroTick();
await nextMicroTick();
expect(fixture.innerHTML).toBe(
"<p><p><p><span>1</span><span>2</span><span>3</span></p></p></p>"
);
// trigger a re-rendering from C, which will remap its new fiber
c!.state.fromC += 10;
// trigger a re-rendering from B, which will remap its new fiber as well
b!.state.fromB += 10;
await nextTick();
// at this point, all re-renderings should have been done correctly, and
// the root fiber (A) counter should have been reset to 0, so the DOM should
// have been patched with the updated version of each component
expect(fixture.innerHTML).toBe(
"<p><p><p><span>11</span><span>12</span><span>13</span></p></p></p>"
);
});
test("concurrent renderings scenario 15", async () => {
let b: B | undefined = undefined;
let c: C | undefined = undefined;
class C extends Component {
static template = xml`
<p>
<span t-esc="props.fromA"/>
<span t-esc="props.fromB"/>
<span t-esc="state.fromC"/>
</p>`;
state = useState({ fromC: 3 });
constructor(parent, props) {
super(parent, props);
c = this;
}
}
class B extends Component {
static template = xml`<p><C fromB="state.fromB" fromA="props.fromA"/></p>`;
static components = { C };
constructor(parent, props) {
super(parent, props);
b = this;
}
state = useState({ fromB: 2 });
}
class A extends Component {
static template = xml`<p><B fromA="state.fromA"/></p>`;
static components = { B };
state = useState({ fromA: 1 });
}
const a = new A();
await a.mount(fixture);
expect(fixture.innerHTML).toBe(
"<p><p><p><span>1</span><span>2</span><span>3</span></p></p></p>"
);
// trigger a re-rendering of the whole tree
a.state.fromA += 10;
// wait enough for the whole tree to be re-rendered, but not patched yet
await nextMicroTick();
await nextMicroTick();
await nextMicroTick();
await nextMicroTick();
await nextMicroTick();
expect(fixture.innerHTML).toBe(
"<p><p><p><span>1</span><span>2</span><span>3</span></p></p></p>"
);
// trigger a re-rendering from C, which will remap its new fiber
c!.state.fromC += 10;
// trigger a re-rendering from B, which will remap its new fiber as well
b!.state.fromB += 10;
// simulate a flush (nothing should have changed as no fiber should have its
// counter to 0)
Component.scheduler.flush();
expect(fixture.innerHTML).toBe(
"<p><p><p><span>1</span><span>2</span><span>3</span></p></p></p>"
);
// wait a bit and simulate another flush (we expect nothing to change as well)
await nextMicroTick();
Component.scheduler.flush();
expect(fixture.innerHTML).toBe(
"<p><p><p><span>1</span><span>2</span><span>3</span></p></p></p>"
);
await nextTick();
expect(fixture.innerHTML).toBe(
"<p><p><p><span>11</span><span>12</span><span>13</span></p></p></p>"
);
});
test("concurrent renderings scenario 16", async () => {
expect.assertions(4);
let b: B | undefined = undefined;
let c: C | undefined = undefined;
class D extends Component {
static template = xml`<ul>DDD</ul>`;
async willStart() {
await nextTick();
await nextTick();
}
}
class C extends Component {
static template = xml`
<p>
<span t-esc="props.fromA"/>
<span t-esc="props.fromB"/>
<span t-esc="state.fromC"/>
<D t-if="state.fromC === 13"/>
</p>`;
static components = { D };
state = { fromC: 3 }; // not reactive
constructor(parent, props) {
super(parent, props);
c = this;
}
}
class B extends Component {
static template = xml`<p><C fromB="state.fromB" fromA="props.fromA"/></p>`;
static components = { C };
constructor(parent, props) {
super(parent, props);
b = this;
}
state = useState({ fromB: 2 });
}
class A extends Component {
static template = xml`<p><B fromA="state.fromA"/></p>`;
static components = { B };
state = useState({ fromA: 1 });
}
const a = new A();
await a.mount(fixture);
expect(fixture.innerHTML).toBe(
"<p><p><p><span>1</span><span>2</span><span>3</span></p></p></p>"
);
// trigger a re-rendering of the whole tree
a.state.fromA += 10;
// wait enough for the whole tree to be re-rendered, but not patched yet
await nextMicroTick();
await nextMicroTick();
await nextMicroTick();
await nextMicroTick();
await nextMicroTick();
expect(fixture.innerHTML).toBe(
"<p><p><p><span>1</span><span>2</span><span>3</span></p></p></p>"
);
// trigger a re-rendering from C, which will remap its new fiber
c!.state.fromC += 10;
const prom = c!.render().then(() => {
expect(fixture.innerHTML).toBe(
"<p><p><p><span>11</span><span>12</span><span>13</span><ul>DDD</ul></p></p></p>"
);
});
// trigger a re-rendering from B, which will remap its new fiber as well
b!.state.fromB += 10;
await nextTick();
// at this point, C rendering is still pending, and nothing should have been
// updated yet.
expect(fixture.innerHTML).toBe(
"<p><p><p><span>1</span><span>2</span><span>3</span></p></p></p>"
);
await prom;
});
test("change state and call manually render: no unnecessary rendering", async () => {
class Widget extends Component {
static template = xml`<div><t t-esc="state.val"/></div>`;