1654969 - Make FastMarshaler always use MSHLFLAGS_NOPING

Status: RESOLVED FIXED

(Core :: IPC: MSCOM, task)

Description

[James Teh [:Jamie]]

Currently, FastMarshaler (used by mscom::Interceptor and a11y::HandlerProvider) uses MSHLFLAGS_NOPING to disable COM garbage collection only if the caller isn't from an external process. The rationale is that we don't need it for calls from the parent process because our content processes can't outlive our parent process, so we don't have to worry about parent process references not being freed.

I propose changing this so that MSHLFLAGS_NOPING is always used, regardless of the calling process. There are three reasons:

1. There seem to be cases where IsCallerExternalProcess() returns true even when marshaling for a COM query in the MTA. I ran JAWS (which is all in-process) and was seeing it return true. In a debug session, I saw this occur when COM was querying for remote interfaces from the parent process, but strangely, I also saw it when UIA Core was calling get_accFocus in-process, which... doesn't make sense.
2. After bug 1627084, we pre-build a11y handler payloads on the main thread for bulk fetch calls. That will marshal interceptors. However, IsCallerExternalProcess() can't work in that case because it's not running on the thread on which the COM call is being handled.
3. If MSHLFLAGS_NOPING is used, Release calls from remote clients are never sent to the server. So, as soon as we use NOPING for our parent process, we're already going to leak references, even if we don't use NOPING for external callers. Put another way, as soon as we use NOPING for one caller, we may as well use it for all callers because COM pinging will never release the object anyway.

Comment 1

[James Teh [:Jamie]]

Attachment: Bug 1654969: Always disable COM pings for mscom::FastMarshaler (and thus mscom::Interceptor).

Previously, we only did this when IsCallerExternalProcess() returned false.
There are three reasons for changing this:

1. There seem to be cases where IsCallerExternalProcess() returns true even when marshaling for a COM query in the MTA.
2. After bug 1627084, we pre-build a11y handler payloads on the main thread for bulk fetch calls. That will marshal interceptors. However, IsCallerExternalProcess() can't work in that case because it's not running on the thread on which the COM call is being handled.
3. If MSHLFLAGS_NOPING is used, Release calls from remote clients are never sent to the server. So, as soon as we use NOPING for our parent process, we're already going to leak references, even if we don't use NOPING for external callers. Put another way, as soon as we use NOPING for one caller, we may as well use it for all callers because COM pinging will never release the object anyway.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8274426a159c
Always disable COM pings for mscom::FastMarshaler (and thus mscom::Interceptor). r=aklotz

Comment 3

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/8274426a159c