Closed Bug 1707954 Opened 4 years ago Closed 4 years ago

Make ProcessRuntime guarantee that all non-main threads are implicit members of the MTA

Categories

(Core :: IPC: MSCOM, task)

Product:
Core
Component:
IPC: MSCOM
Type:
task

Tracking

()

Status:
RESOLVED FIXED
Milestone:
91 Branch
Tracking Flags:
Tracking Status
firefox91 --- fixed

People

(Reporter: bugzilla, Assigned: bugzilla)

References

Details

Attachments

(4 files)

Bug 1707954: Part 1 - Change mscom::ProcessRuntime to ensure MTA creation during startup; r=Jamie
48 bytes, text/x-phabricator-request
Details | Review
Bug 1707954: Part 2 - Update Launcher Process with mscom::ProcessRuntime changes; r=Jamie
48 bytes, text/x-phabricator-request
Details | Review
Bug 1707954: Part 3 - Add eager MTA creation to mscom::EnsureMTA; r=Jamie
48 bytes, text/x-phabricator-request
Details | Review
Bug 1707954: Part 4 - Build fixes to reflect EnsureMTA changes; r=Jamie
48 bytes, text/x-phabricator-request
Details | Review

I want to make it so that we can eventually ban CoInitialize* and OleInitialize outside of ProcessRuntime. Let's ensure that we always have the MTA up and running, so that all threads that do not explicitly declare their apartment membership are implicit members of the MTA.

This patch does the following:

  • General cleanup:
    • More explicit restrictions of how/when the various constructors are available.
    • InitializeSecurity is made static, since it doesn't really manipulate instance variables.
    • We move some logic for resolving CoInitializeEx flags into GetDesiredApartmentType.
    • We also try to set COINIT_SPEED_OVER_MEMORY for 64-bit builds with more than 4GB of RAM.
      I'm not sure how useful that is, but I thought it might be interesting to try.
  • Addition of PostInit:
    • This doesn't do anything at the moment, but since I'm already making a bunch
      of changes, I wanted to add this too. PostInit is a static method that
      is invoked once the ProcessRuntime is finished initializing, and, when
      present, the sandbox is fully enabled.
  • We call EnsureMTA's default constructor to eagerly bring up the MTA. This
    causes all background threads to implicitly become members of the MTA, which
    means that we can eliminate CoInitializeEx calls throughout our codebase,
    since they're slow and most developers do not have a clear understanding of
    what those functions actually do.
    • This also simplifies the COM initialization in sandboxed content processes
      with Win32K lockdown, since if our main thread is in the implicit MTA, we
      can immediately initialize ourselves without needing to punt that work
      over to the persistent MTA thread.

Given the changes in part 1, we must now use the ProcessCategory variant of
ProcessRuntime's constructors.

Depends on D113560

  • We make EnsureMTA's default constructor private, and ProcessRuntime a friend.
    ProcessRuntime calls this to eagerly create the MTA.
  • The default constructor uses the new-ish CoIncrementMTAUsage to create the
    MTA without requiring a dedicated thread (when available). Otherwise we
    fall back to the traditional method. In the latter case, we synchronously
    wait for the initialization to complete so that we are guaranteed to have
    an MTA when we return.
  • Some minor refactoring to make it easier to do the sync wait in the
    default constructor. I also renamed a couple of things just to make them
    more clear.

Depends on D113561

Now that we always have an MTA active, we don't need to explicitly try to
start it anymore. These locations in our source were doing so, which is now
not only redundant, but fails (since EnsureMTA's default constructor is now
private).

Depends on D113562

Pushed by aklotz@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/fb4829011104 Part 1 - Change mscom::ProcessRuntime to ensure MTA creation during startup; r=Jamie https://hg.mozilla.org/integration/autoland/rev/f72b810472fd Part 2 - Update Launcher Process with mscom::ProcessRuntime changes; r=Jamie https://hg.mozilla.org/integration/autoland/rev/e1b37839487b Part 3 - Add eager MTA creation to mscom::EnsureMTA; r=Jamie https://hg.mozilla.org/integration/autoland/rev/fa23f9293250 Part 4 - Build fixes to reflect EnsureMTA changes; r=Jamie
Flags: needinfo?(aklotz)
Pushed by aklotz@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/49406bdac5ec Part 1 - Change mscom::ProcessRuntime to ensure MTA creation during startup; r=Jamie https://hg.mozilla.org/integration/autoland/rev/55586d8f7bf4 Part 2 - Update Launcher Process with mscom::ProcessRuntime changes; r=Jamie https://hg.mozilla.org/integration/autoland/rev/fd52d202d10b Part 3 - Add eager MTA creation to mscom::EnsureMTA; r=Jamie https://hg.mozilla.org/integration/autoland/rev/7cb0db27236c Part 4 - Build fixes to reflect EnsureMTA changes; r=Jamie
Backout by mlaza@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d00ac5eba3b0 Backed out 4 changesets for causing bc failures in ClearOnShutdown.cpp.

Backed out 4 changesets (Bug 1707954) for causing bc failures in ClearOnShutdown.cpp.

Backout link

Push with failures

Failure Log

Flags: needinfo?(aklotz)

There are also xpcshell failures on test_launch_without_hang.js

Failure Log

Flags: needinfo?(aklotz)
Pushed by aklotz@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d1c08a9fc957 Part 1 - Change mscom::ProcessRuntime to ensure MTA creation during startup; r=Jamie https://hg.mozilla.org/integration/autoland/rev/c419279fff66 Part 2 - Update Launcher Process with mscom::ProcessRuntime changes; r=Jamie https://hg.mozilla.org/integration/autoland/rev/4e833faa1996 Part 3 - Add eager MTA creation to mscom::EnsureMTA; r=Jamie https://hg.mozilla.org/integration/autoland/rev/224d902faaf8 Part 4 - Build fixes to reflect EnsureMTA changes; r=Jamie
Depends on: 1716036
Regressions: 1716743
See Also: → 1719212
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: