Closed Bug 1310841 Opened 3 years ago Closed 3 years ago

Make MSCOM registration use CoGetClassObject

Categories

(Core :: IPC, defect)

Unspecified
Windows
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: aklotz, Assigned: aklotz)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

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 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 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+
Attachment #8801904 - Attachment is obsolete: true
Attachment #8804450 - Attachment is obsolete: true
Attachment #8804450 - Flags: review?(gps)
WTF MozReview!
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
https://hg.mozilla.org/mozilla-central/rev/1eed5e6b3f51
https://hg.mozilla.org/mozilla-central/rev/b59a4d7a3d2c
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.