1819862 - Reduce warnings from Msg_CleanupPendingLoadState about "actor cannot send"

Status: RESOLVED FIXED

(Core :: DOM: Content Processes, task, P3)

Description

[Jens Stutte [:jstutte]]

The nsDocShellLoadState::~nsDocShellLoadState tries to send a cleanup message to the parent. In case we find ourselves inside a client process whose actor has been already closed, this will generate a very frequent WARNING log message like:

[task 2023-03-02T08:28:33.615Z] 08:28:33     INFO - PID 1303 | [Child 1546, Main Thread] WARNING: IPC message 'PContent::Msg_CleanupPendingLoadState' discarded: actor cannot send: file /builds/worker/checkouts/gecko/ipc/glue/ProtocolUtils.cpp:504

We should find out if this is really worrisome enough to spam our logs like this (then we should fight the root cause) or just check the actor for CanSend before sending, assuming that a closed actor already triggered all necessary cleanup on the parent's side.

Comment 1

[Jens Stutte [:jstutte]]

Artur, would you mind to take a look? Can you try to confirm that "a closed actor already triggered all necessary cleanup on the parent's side." ?

Comment 2

[Artur Iunusov]

looking

Comment 3

[Artur Iunusov]

It seems that the purpose of this message is only mPendingLoadStates cleanup.
mPendingLoadStates.Clear() is called by ContentParent::ActorDestroy() which is called by IProtocol::DestroySubtree which is called during Shutdown (OnChannelClose/OnChannelError)
So, we might consider using CanSend(), as the LinkStatus changes during the process, described above.

This code seems unused and can be removed: https://searchfox.org/mozilla-central/source/ipc/glue/ProtocolUtils.cpp#524

I proposed a patch

Comment 4

[Artur Iunusov]

Attachment: Bug 1819862: Reduce warnings from Msg_CleanupPendingLoadState about "actor cannot send", r=jstutte

Comment 6

[Andrew McCreight [:mccr8]]

This is the 9th most common cause of warning spam on m-c MacOS tests.

Comment 7

[Andrew McCreight [:mccr8]]

Here's the 5 most common tests where this occurs. The first 1 isn't so surprising, but it does seem a little odd for so many of them to happen in a random test like that.

   522 -        Shutdown
   300 -        dom/security/test/referrer-policy/browser_referrer_disallow_cross_site_relaxing.js
   276 -        /webdriver/tests/bidi/network/add_intercept/url_patterns.py
   142 -        /webdriver/tests/bidi/browsing_context/locate_nodes/invalid.py
   132 -        /webdriver/tests/classic/forward/user_prompts.py

Comment 8

[Andrew McCreight [:mccr8]]

I added this line to `~nsDocShellLoadState()':

NS_ASSERTION(ContentChild::GetSingleton()->CanSend(), "can't send!!!");

Then I ran browser_referrer_disallow_cross_site_relaxing.js.

The stacks I looked at were like this:

nsDocShellLoadState::~nsDocShellLoadState() (docshell/base/nsDocShellLoadState.cpp:250)
nsDocShellLoadState::Release() (nsDocShellLoadState.h:47)
mozilla::net::DocumentChannel::~DocumentChannel() DocumentChannel.h:87)
mozilla::net::DocumentChannelChild::~DocumentChannelChild() (netwerk/ipc/DocumentChannelChild.cpp:60)
mozilla::net::DocumentChannelChild::Release() (netwerk/ipc/DocumentChannelChild.cpp:45)

and then above that, this is the deferred finalizer from a shutdown GC triggered by the shutdown CC. So it kind of looks like we're leaking DocumentChannelChild until shutdown via JS? Does that make sense?

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/054e4dcfb95b
Reduce warnings from Msg_CleanupPendingLoadState about "actor cannot send", r=jstutte

Comment 10

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/054e4dcfb95b