Make MSCOM registration use CoGetClassObject

RESOLVED FIXED in Firefox 52

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: aklotz, Assigned: aklotz)

Tracking

(Blocks: 1 bug)

Trunk
mozilla52
Unspecified
Windows
Points:
---

Firefox Tracking Flags

(firefox52 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

2 years ago
I'm seeing occasional crashes in COM apartment shutdown when Windows a11y+e10s is enabled. I think this is because our RegisteredProxy objects are releasing COM class objects before the COM runtime is done with them.

I think that if we modify this code to use CoGetClassObject instead of DllGetClassObject, the COM runtime will retain an additional reference to the class object and thus if necessary will be able to hang onto the DLL even after we have released it ourselves.

I have not been able to reproduce these crashes since I committed these changes to my local repo.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 3

2 years ago
mozreview-review
Comment on attachment 8801904 [details]
Bug 1310841: Modify build rules to allow specification of a manifest for Windows DLLs;

https://reviewboard.mozilla.org/r/86494/#review86588

This looks OK. Ideally this code would be in the Python part of the build system. But since the existing code isn't in Python, I won't scope bloat your work.
Attachment #8801904 - Flags: review?(gps) → review+

Comment 4

2 years ago
mozreview-review
Comment on attachment 8801905 [details]
Bug 1310841: Make mscom registration use CoGetClassObject so that COM will retain a reference to the proxy dll;

https://reviewboard.mozilla.org/r/86496/#review86618

::: ipc/mscom/ActivationContext.cpp:26
(Diff revision 1)
> +  actCtx.lpResourceName = MAKEINTRESOURCE(2);
> +  actCtx.hModule = aLoadFromModule;
> +
> +  mActCtx = ::CreateActCtx(&actCtx);
> +  if (mActCtx == INVALID_HANDLE_VALUE) {
> +    return;

Lets warn here, afaict this generally shouldn't fail.

::: ipc/mscom/ActivationContext.cpp:31
(Diff revision 1)
> +    return;
> +  }
> +  if (!::ActivateActCtx(mActCtx, &mActivationCookie)) {
> +    ::ReleaseActCtx(mActCtx);
> +    mActCtx = INVALID_HANDLE_VALUE;
> +    return;

nit - pointless return statement.
Attachment #8801905 - Flags: review?(jmathies) → review+
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Attachment #8801904 - Attachment is obsolete: true
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Attachment #8804450 - Attachment is obsolete: true
Attachment #8804450 - Flags: review?(gps)
(Assignee)

Comment 7

2 years ago
WTF MozReview!
(Assignee)

Comment 8

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/1eed5e6b3f51c41cbb05886dd4fdef3a4f33d7af
Bug 1310841: Modify build rules to allow specification of a manifest for Windows DLLs; r=gps

https://hg.mozilla.org/integration/mozilla-inbound/rev/b59a4d7a3d2c57c33aae29a8844100f2223ab5bc
Bug 1310841: Make mscom registration use CoGetClassObject so that COM will retain a reference to the proxy dll; r=jimm

Comment 9

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/1eed5e6b3f51
https://hg.mozilla.org/mozilla-central/rev/b59a4d7a3d2c
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox52: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.