(In reply to Bas Schouten (:bas.schouten) from comment #4) > As Nika mentioned on here, there being tasks in the queue while we're not doing anything is expected. Idle tasks are the most likely thing in the queue here. However we should be waking up when we are ready to process these. (Olli would know better exactly how this work, but I think it's related with receiving the idle token) OK, that makes it most likely to not matter if we have tasks in the queue. > When we are shutting down all idle tasks should be free to be executed though. And when we're not shutting down pending idle tasks should never be causing a deadlock, otherwise they shouldn't be idle tasks :-). > > As Nika said though, some of this code has changed over the years and the decoupling means we could have accidentally created a situation where the CV is not being signalled. We could try signalling it somewhat more aggressively during manipulation of ```mMainThreadTasks```. The downside would be slightly more wakeups potentially, but I don't think it would do any real harm. The symptom we see is that apparently the [`ContentChild::RecvShutdown`](https://searchfox.org/mozilla-central/rev/962a843f6d96283c45162c788dc72bf217fca7df/dom/ipc/ContentChild.cpp#2991) function is never executed. IIUC that is supposed to happen executing a `MessageChannel::MessageTask` that wraps `ContentChild::OnMessage` and is dispatched to the main thread. However, re-reading the `ContentParent::MaybeBeginShutDown` code it seems to me, that if `aSendShutDown` is `false` we can end up starting our timer without having sent the `ContentParent::SendShutdown` at all ( [at least here in this function](https://hg.mozilla.org/mozilla-central/log/4fbf0aea0b4a72bdd461e0643d9da7d85367ab9d/dom/ipc/ContentParent.cpp) ) which would obviously perfectly explain what is going on. That function and flag has been introduced by bug 1661364 and it seems to me we should either decide to die or not, but not halfway. And also [`ContentParent::MaybeAsyncSendShutDownMessage`](https://searchfox.org/mozilla-central/rev/962a843f6d96283c45162c788dc72bf217fca7df/dom/ipc/ContentParent.cpp#1790-1792) can still decide to not send the message. I need to parse this a bit better, but it seems to me we might end up having started the timer on the parent side without ever even have scheduled the parent side task that is supposed to send the shutdown message.
Bug 1837467 Comment 5 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 Bas Schouten (:bas.schouten) from comment #4) > As Nika mentioned on here, there being tasks in the queue while we're not doing anything is expected. Idle tasks are the most likely thing in the queue here. However we should be waking up when we are ready to process these. (Olli would know better exactly how this work, but I think it's related with receiving the idle token) OK, that makes it most likely to not matter if we have tasks in the queue. > When we are shutting down all idle tasks should be free to be executed though. And when we're not shutting down pending idle tasks should never be causing a deadlock, otherwise they shouldn't be idle tasks :-). > > As Nika said though, some of this code has changed over the years and the decoupling means we could have accidentally created a situation where the CV is not being signalled. We could try signalling it somewhat more aggressively during manipulation of ```mMainThreadTasks```. The downside would be slightly more wakeups potentially, but I don't think it would do any real harm. The symptom we see is that apparently the [`ContentChild::RecvShutdown`](https://searchfox.org/mozilla-central/rev/962a843f6d96283c45162c788dc72bf217fca7df/dom/ipc/ContentChild.cpp#2991) function is never executed. IIUC that is supposed to happen executing a `MessageChannel::MessageTask` that wraps `ContentChild::OnMessage` and is dispatched to the main thread. However, re-reading the `ContentParent::MaybeBeginShutDown` code it seems to me, that if `aSendShutDown` is `false` we can end up starting our timer without having sent the `ContentParent::SendShutdown` at all ( [at least here in this function](https://hg.mozilla.org/mozilla-central/log/4fbf0aea0b4a72bdd461e0643d9da7d85367ab9d/dom/ipc/ContentParent.cpp) ) which would obviously perfectly explain what is going on. That function and flag has been introduced by bug 1661364 and it seems to me we should either decide to die or not, but not halfway. And also [`ContentParent::MaybeAsyncSendShutDownMessage`](https://searchfox.org/mozilla-central/rev/962a843f6d96283c45162c788dc72bf217fca7df/dom/ipc/ContentParent.cpp#1790-1792) can still decide to not send the message. I need to parse this a bit better, but it seems to me we might end up having started the timer on the parent side without ever even have scheduled the parent side task that is supposed to send the shutdown message. Note: That would make it be in the correct component, of course.
(In reply to Bas Schouten (:bas.schouten) from comment #4) > As Nika mentioned on here, there being tasks in the queue while we're not doing anything is expected. Idle tasks are the most likely thing in the queue here. However we should be waking up when we are ready to process these. (Olli would know better exactly how this work, but I think it's related with receiving the idle token) OK, that makes it most likely to not matter if we have tasks in the queue. > When we are shutting down all idle tasks should be free to be executed though. And when we're not shutting down pending idle tasks should never be causing a deadlock, otherwise they shouldn't be idle tasks :-). > > As Nika said though, some of this code has changed over the years and the decoupling means we could have accidentally created a situation where the CV is not being signalled. We could try signalling it somewhat more aggressively during manipulation of ```mMainThreadTasks```. The downside would be slightly more wakeups potentially, but I don't think it would do any real harm. The symptom we see is that apparently the [`ContentChild::RecvShutdown`](https://searchfox.org/mozilla-central/rev/962a843f6d96283c45162c788dc72bf217fca7df/dom/ipc/ContentChild.cpp#2991) function is never executed. IIUC that is supposed to happen executing a `MessageChannel::MessageTask` that wraps `ContentChild::OnMessage` and is dispatched to the main thread. However, re-reading the `ContentParent::MaybeBeginShutDown` code it seems to me, that if `aSendShutDown` is `false` we can end up starting our timer without having sent the `ContentParent::SendShutdown` at all ( [at least here in this function](https://searchfox.org/mozilla-central/rev/962a843f6d96283c45162c788dc72bf217fca7df/dom/ipc/ContentParent.cpp#1769) ) which would obviously perfectly explain what is going on. That function and flag has been introduced by bug 1661364 and it seems to me we should either decide to die or not, but not halfway. And also [`ContentParent::MaybeAsyncSendShutDownMessage`](https://searchfox.org/mozilla-central/rev/962a843f6d96283c45162c788dc72bf217fca7df/dom/ipc/ContentParent.cpp#1790-1792) can still decide to not send the message. I need to parse this a bit better, but it seems to me we might end up having started the timer on the parent side without ever even have scheduled the parent side task that is supposed to send the shutdown message. Note: That would make it be in the correct component, of course.