Bug 1741869 Comment 15 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

(In reply to Andrew Sutherland [:asuth] (he/him) from comment #14)
> The comment 1 stack combined with the use of FileReader in the test makes me suspect that the problem is that the FileReader's StrongWorkerRef [callback invoking Shutdown](https://searchfox.org/mozilla-central/rev/a3bddeeb9436fc7d0efba7adbd5c8ab31c2750cc/dom/file/FileReader.cpp#775-777) is failing to call `ClearProgressEventTimer()` like [Abort() does](https://searchfox.org/mozilla-central/rev/a3bddeeb9436fc7d0efba7adbd5c8ab31c2750cc/dom/file/FileReader.cpp#775-777).  (Shutdown seems to be a specialized variant of Abort.)  This would explain the rogue timer event.

Thanks for the analysis! This would make it become a duplicate of bug 1650214 then. A fix is already up for review there.

Still I am a bit unsure about the expected lifecycle of the `WorkerGlobalScopeBase` object in relation to the `WorkerPrivate` one. It is not clear to me why we would want to include `WorkerGlobalScopeBase::mWorkerPrivate` [in the cycle collection here at all](https://searchfox.org/mozilla-central/rev/3c9369510cb883b9f08d5f9641e1233d2142f025/dom/workers/WorkerScope.cpp#186-187,195-196) if we do not want it to happen in the end. Doing this through a weak raw pointer reference makes it kind of unpredictable, anyway. So even if the canceling of the timer will fix this specific one, we should continue to reason about bug 1744025, IMHO.
(In reply to Andrew Sutherland [:asuth] (he/him) from comment #14)
> The comment 1 stack combined with the use of FileReader in the test makes me suspect that the problem is that the FileReader's StrongWorkerRef [callback invoking Shutdown](https://searchfox.org/mozilla-central/rev/a3bddeeb9436fc7d0efba7adbd5c8ab31c2750cc/dom/file/FileReader.cpp#775-777) is failing to call `ClearProgressEventTimer()` like [Abort() does](https://searchfox.org/mozilla-central/rev/a3bddeeb9436fc7d0efba7adbd5c8ab31c2750cc/dom/file/FileReader.cpp#775-777).  (Shutdown seems to be a specialized variant of Abort.)  This would explain the rogue timer event.

Thanks for the analysis! This would make it become a duplicate of bug 1650214 then, I should have realized this earlier, sorry. A fix is already up for review there.

Still I am a bit unsure about the expected lifecycle of the `WorkerGlobalScopeBase` object in relation to the `WorkerPrivate` one. It is not clear to me why we would want to include `WorkerGlobalScopeBase::mWorkerPrivate` [in the cycle collection here at all](https://searchfox.org/mozilla-central/rev/3c9369510cb883b9f08d5f9641e1233d2142f025/dom/workers/WorkerScope.cpp#186-187,195-196) if we do not want it to happen in the end. Doing this through a weak raw pointer reference makes it kind of unpredictable, anyway. So even if the canceling of the timer will fix this specific one, we should continue to reason about bug 1744025, IMHO.
(In reply to Andrew Sutherland [:asuth] (he/him) from comment #14)
> The comment 1 stack combined with the use of FileReader in the test makes me suspect that the problem is that the FileReader's StrongWorkerRef [callback invoking Shutdown](https://searchfox.org/mozilla-central/rev/a3bddeeb9436fc7d0efba7adbd5c8ab31c2750cc/dom/file/FileReader.cpp#775-777) is failing to call `ClearProgressEventTimer()` like [Abort() does](https://searchfox.org/mozilla-central/rev/a3bddeeb9436fc7d0efba7adbd5c8ab31c2750cc/dom/file/FileReader.cpp#775-777).  (Shutdown seems to be a specialized variant of Abort.)  This would explain the rogue timer event.

Thanks for the analysis! This would make it become a duplicate of bug 1650214 then, I should have realized this earlier, sorry. A fix is already up for review there.

Still I am a bit unsure about the expected lifecycle of the `WorkerGlobalScopeBase` object in relation to the `WorkerPrivate` one. It is not clear to me why we would want to include `WorkerGlobalScopeBase::mWorkerPrivate` [in the cycle collection here at all](https://searchfox.org/mozilla-central/rev/3c9369510cb883b9f08d5f9641e1233d2142f025/dom/workers/WorkerScope.cpp#186-187,195-196) if we do not want it to happen in the end. Doing this through a weak raw pointer reference makes it kind of unpredictable, anyway. So even if the canceling of the timer will fix this specific one, we should continue to reason about bug 1744025 in general and the nature of every single `WorkerPrivate` reference (weak or strong) in particular, IMHO.

Back to Bug 1741869 Comment 15