1400344 - Stop initializing single-threaded COM in the content processes

Status: RESOLVED FIXED

(Core :: IPC: MSCOM, defect, P1)

Description

[Alex Gaynor [:Alex_Gaynor]]

Example stack:

    win32u!NtUserMessageCall
    USER32!RealDefWindowProcWorker+0x1ba
    USER32!RealDefWindowProcW+0x52
    UxTheme!_ThemeDefWindowProc+0x559
    UxTheme!ThemeDefWindowProcW+0x11
    USER32!DefWindowProcW+0x1b7
    combase!OleMainThreadWndProc+0x25
    USER32!UserCallWinProcCheckWow+0x280
    USER32!DispatchClientMessage+0x9c
    USER32!_fnINOUTNCCALCSIZE+0x3b
    ntdll!KiUserCallbackDispatcherContinue
    win32u!NtUserCreateWindowEx+0x14
    USER32!VerNtUserCreateWindowEx+0x212
    USER32!CreateWindowInternal+0x1b5
    USER32!CreateWindowExW+0x82
    combase!InitMainThreadWnd+0x56
    combase!ThreadFirstInitialize+0x136
    combase!_CoInitializeEx+0x1db
    combase!CoInitializeEx+0x36
    xul!mozilla::mscom::COMApartmentRegion<2>::{ctor}+0x11
    xul!mozilla::mscom::MainThreadRuntime::MainThreadRuntime+0x6a
    xul!mozilla::dom::ContentProcess::{ctor}+0x36
    xul!XRE_InitChildProcess+0x67c


This is blocked by us

Comment 1

[(No longer employed by Mozilla) Aaron Klotz]

Essentially we don't want to initialize an STA in content, since the STA uses a hidden window and requires a message pump.

OTOH, we still need to properly configure COM, so just removing MainThreadRuntime is a non-starter.

I will probably need to modify that code to configure itself inside MTA instead... but we probably don't want the startup thread itself to be MTA.

In other words, this isn't a trivial fix and some thought needs to go into this. :-)

Comment 2

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: WIP

This is a WIP patch that moves COM init to the MTA in content processes.

This is not yet a complete solution because we still need to audit our code to determine whether there is anything in content that might attempt COM on the main thread.

Comment 3

[(No longer employed by Mozilla) Aaron Klotz]

Here's a try push with the above patch. Let's see if any tests break.

Comment 4

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Bug 1400344: Rename mscom::MainThreadRuntime to mscom::ProcessRuntime and make it aware of Win32k lockdown and of multiple instantiations; r=Jamie

This patch takes care of a bunch of issues and does some cleanup:

• We rename mscom::MainThreadRuntime to mscom::ProcessRuntime, as the latter
  is a more accurate name going forward.
• We make ProcessRuntime aware of the Win32k Lockdown process mitigation
  policy. When Win32k is disabled, we perform process-wide COM initialization
  in the multi-threaded apartment (since we cannot create an STA window).
• We refactor the mscom apartment region stuff to enable the Win32k lockdown
  pieces in ProcessRuntime.
• We move some Gecko-specific stuff into MOZILLA_INTERNAL_API guards so that
  ProcessRuntime is usable outside of xul.dll (I will be needing it for the
  launcher process).
• Another thing that might happen with the launcher process is that, under
  error conditions in the launcher, we create a ProcessRuntime object on a
  background thread for the purposes of telemetry logging, but we also allow
  the main thread to proceed to start as the browser. This could result in a
  scenario where the main thread, as the browser process, is attempting to
  instantiate its ProcessRuntime and ends up racing with the launcher process's
  telemetry thread which has its own ProcessRuntime. To account for this
  situation, we add mutual exclusion to the process-wide initialization code.
• We clean up ProcessRuntime::InitializeSecurity by using Vector to set up
  the EXPLICIT_ACCESS entries.
• We remove mscom::MainThreadClientInfo and replace it with a direct call to
  CoGetCallerTID
• We revise all references to this class to use the new name.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2d4b8d90cbd7
Rename mscom::MainThreadRuntime to mscom::ProcessRuntime and make it aware of Win32k lockdown and of multiple instantiations; r=Jamie

Comment 6

[Dorel Luca [:dluca]]

Backed out changeset fe264afc03aa (bug 1511224) for Fetch failures. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=228441744&repo=autoland&lineNumber=777

Push with faiures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=2d4b8d90cbd789525ae9a8829b21c3cecac5d594

Backout:
https://hg.mozilla.org/integration/autoland/rev/179d3ee81e2d292092a06761bbd44eb6ea78daa8

Comment 7

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dbd7b92caef0
Backed out changeset 2d4b8d90cbd7 for Spider monkey failrues. CLOSED TREE

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/13854b5d426c
Rename mscom::MainThreadRuntime to mscom::ProcessRuntime and make it aware of Win32k lockdown and of multiple instantiations; r=Jamie

Comment 9

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/13854b5d426c