1310841 - Make MSCOM registration use CoGetClassObject

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[(No longer employed by Mozilla) Aaron Klotz]

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 1

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Bug 1310841: Modify build rules to allow specification of a manifest for Windows DLLs;

Review commit: https://reviewboard.mozilla.org/r/86494/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/86494/

Comment 2

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Bug 1310841: Make mscom registration use CoGetClassObject so that COM will retain a reference to the proxy dll;

Review commit: https://reviewboard.mozilla.org/r/86496/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/86496/

Comment 3

[Gregory Szorc [:gps]]

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.

Comment 4

[Jim Mathies [:jimm]]

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.

Comment 5

[(No longer employed by Mozilla) Aaron Klotz]

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

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/86496/diff/1-2/

Comment 6

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Bug 1310841: Modify build rules to allow specification of a manifest for Windows DLLs;

Review commit: https://reviewboard.mozilla.org/r/88414/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/88414/

Comment 7

[(No longer employed by Mozilla) Aaron Klotz]

WTF MozReview!

Comment 8

[(No longer employed by Mozilla) Aaron Klotz]

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

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1eed5e6b3f51
https://hg.mozilla.org/mozilla-central/rev/b59a4d7a3d2c