Closed Bug 1654969 Opened 2 years ago Closed 2 years ago

Make FastMarshaler always use MSHLFLAGS_NOPING

Categories

(Core :: IPC: MSCOM, task)

task

Tracking

()

RESOLVED FIXED
81 Branch
Tracking Status
firefox81 --- fixed

People

(Reporter: Jamie, Assigned: Jamie)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.

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.
See Also: → 1434822
Summary: Make Fastmarshaler always use MSHLFLAGS_NOPING → Make FastMarshaler always use MSHLFLAGS_NOPING
Pushed by jteh@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8274426a159c
Always disable COM pings for mscom::FastMarshaler (and thus mscom::Interceptor). r=aklotz
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 81 Branch
You need to log in before you can comment on or make changes to this bug.