Which @angular/* package(s) are the source of the bug?
zone.js
Is this a regression?
Yes
Description
Currently, Zone.js patches setTimeout and keeps a tasksByHandleId map to keep a timerId <-> ZoneTask
relationship. This map appears to be global to each method per window (browser use case) / global instance and not per Zone as it's defined on patchTimer.
Tasks then use the return value from their schedule methods (such as setTimeout) to store this ID in tasksByHandleId. For example, in the case of a root zone with no wrapping it normally just calls
setNative which may invoke setTimeout and we store the timer ID in the map. These IDs are monotonically increasing and therefore are never reused. However, in the case of fake-async-test it starts from 1 and increments it's own counters per task.
If you have a Zone that is using setNative and scheduling using the real browser scheduler and using fake-async-zone it is possible to generate two tasks that are both pending with the same ID because they don't know of each other. For example:
let oldTimerId =
runInFakeAsyncZone(() => {
// Run a timer in such a way that it also returns the same timer ID
// but does not resolve, thus overwriting the taskMap
// Maybe it wasn't fired from an even and has to be flushed or something
oldTimerId = ...
});
// Run in <root>, running later
// timerId == oldTimerId now
// Therefore, `tasksByHandleId` is overwritten to use this ID
let timerId = setTimeout(() => console.log('I should run but never do'), 30000);
runInFakeAsyncZone(() => {
// We changed our mind, cancel the timer
clearInterval(oldTimerId)
// Actually, they shared an ID and `tasksByHandleId` was updated
// to point to the <root> task so `task.zone.cancelTask(task)` is invoked
// but the implementation uses `clearNative` and clears the <root> task
// meaning that the setTimeout never runs
});
In the above psuedo-code, the task that runs in some other zone is overwritten with one that runs the root later because of the shared map. In contexts outside of fake-async-test, it's likely never going to happen
because the IDs are strictly monotonically increasing. But in fake-async-test, this is troublesome because they can overlap which is bad news when using a single global tasksByHandleId
Some thoughts:
- I don't know if this use case is that common but it's at least common in a couple of Google tests that trigger this behaviour
- If overlap is not expected, the task map should probably assert and break early because this was a huge pain to track down as not only is timing sensitive but it's also a read / write race
- If overlap is expected in some cases, we need to somehow handle this
I've not included a full reproduction yet because the exact specifics are kind of hard to encode but I will try and get something up.
Please provide a link to a minimal reproduction of the bug
N/A; coming soon maybe
Please provide the exception or error you saw
No response
Please provide the environment you discovered this bug in (run ng version)
Anything else?
No response
Which @angular/* package(s) are the source of the bug?
zone.js
Is this a regression?
Yes
Description
Currently, Zone.js patches
setTimeoutand keeps atasksByHandleIdmap to keep atimerId<->ZoneTaskrelationship. This map appears to be global to each method per
window(browser use case) /globalinstance and not perZoneas it's defined onpatchTimer.Tasks then use the return value from their schedule methods (such as
setTimeout) to store this ID intasksByHandleId. For example, in the case of a root zone with no wrapping it normally just callssetNativewhich may invokesetTimeoutand we store the timer ID in the map. These IDs are monotonically increasing and therefore are never reused. However, in the case offake-async-testit starts from1and increments it's own counters per task.If you have a
Zonethat is usingsetNativeand scheduling using the real browser scheduler and usingfake-async-zoneit is possible to generate two tasks that are both pending with the same ID because they don't know of each other. For example:In the above psuedo-code, the task that runs in some other zone is overwritten with one that runs the
rootlater because of the shared map. In contexts outside offake-async-test, it's likely never going to happenbecause the IDs are strictly monotonically increasing. But in
fake-async-test, this is troublesome because they can overlap which is bad news when using a single globaltasksByHandleIdSome thoughts:
I've not included a full reproduction yet because the exact specifics are kind of hard to encode but I will try and get something up.
Please provide a link to a minimal reproduction of the bug
N/A; coming soon maybe
Please provide the exception or error you saw
No response
Please provide the environment you discovered this bug in (run
ng version)Anything else?
No response