1310056 - Implement compatibility hack for RPC_E_CANTCALLOUT_ININPUTSYNCCALL

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[(No longer employed by Mozilla) Aaron Klotz]

See https://bugzilla.mozilla.org/show_bug.cgi?id=1297549#c14.

If making a call on a COM proxy from an STA when InSendMessageEx reports that the thread is blocking, the call will fail with RPC_E_CANTCALLOUT_ININPUTSYNCCALL.

This is overkill in our case because our proxies go into the content MTA and don't reenter the chrome STA.

a11y clients can be modified to fix this (and Jamie has already landed one patch to NVDA), but I don't think we can count on that from a compat perspective.

Comment 1

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Part 1 - Enhancements to DLL interceptor

Comment 2

[(No longer employed by Mozilla) Aaron Klotz]

Comment on attachment 8800909 [details] [diff] [review]
Part 1 - Enhancements to DLL interceptor

This patch adds x86 support for prefix groups 1 and 2 (as denoted by the Intel processor manuals) since I don't expect those prefixes to affect operand or address size. We explicitly fail if either group 3 or 4 prefixes are present.

In the future I'd like to do some further work for amd64 as well as generally clean up the detour patcher so that it better exhibits the structure of the instruction set, but I want to land the minimum right now.

Comment 3

[(No longer employed by Mozilla) Aaron Klotz]

https://hg.mozilla.org/integration/mozilla-inbound/rev/92d09cb049f594de283a06e111a2faaf74fbeb21
Bug 1310056: Part 1 - DllInterceptor improvements to support InSendMessageEx; r=m_kato

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=37993046&repo=mozilla-inbound

https://hg.mozilla.org/integration/mozilla-inbound/rev/e151b7a9b0c7d327d5800004d7a5d90474317235

Comment 5

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Part 2: Additional DLL interceptor enhancements

Verified by try push and also running locally on 32-bit and 64-bit Windows 10.

Comment 6

[(No longer employed by Mozilla) Aaron Klotz]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7d456e23a2ca3935fd0555089ba554297236aaef
Bug 1310056: Part 1 - DllInterceptor improvements to support InSendMessageEx; r=m_kato

https://hg.mozilla.org/integration/mozilla-inbound/rev/089321ce84cf731602a9979f3af1d6ecef5c0270
Bug 1310056: Part 2 - Additional DLL interceptor enhancements; r=m_kato

Comment 7

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/7d456e23a2ca
https://hg.mozilla.org/mozilla-central/rev/089321ce84cf

Comment 8

[(No longer employed by Mozilla) Aaron Klotz]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e2e2119e21eb

Comment 9

[(No longer employed by Mozilla) Aaron Klotz]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4c5ef4bff387

Comment 10

[Julien Cristau [:jcristau]]

Mass wontfix for bugs affecting firefox 52.

Comment 11

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Part 3: Detect RPC_E_CANTCALLOUT_ININPUTSYNCCALL errors and lazily enable compatibility hack

This patch is required for JAWS. Initial r? to davidb since he wrote a bunch of this.

If we are running a known AT (except for NVDA, which doesn't suffer from this), we set a special exception handler that lets us intercept RPC_E_CANTCALLOUT_ININPUTSYNCCALL errors. When we see one of these, we enable the compatibility hack.

The hack causes InSendMessageEx to return ISMEX_NOTIFY when invoked by COM.

Comment 13

[David Bolter [:davidb] (NeedInfo me for attention)]

Comment on attachment 8860188 [details] [diff] [review]
Part 3: Detect RPC_E_CANTCALLOUT_ININPUTSYNCCALL errors and lazily enable compatibility hack

Review of attachment 8860188 [details] [diff] [review]:
-----------------------------------------------------------------

r=me
TIL.
Sort of worrying but I think I'm okay with your choice of not doing the hack for Compatibility::UNKNOWN.

::: accessible/windows/msaa/Compatibility.cpp
@@ +47,5 @@
>  ////////////////////////////////////////////////////////////////////////////////
>  // Compatibility
>  ////////////////////////////////////////////////////////////////////////////////
>  
> +static WindowsDllInterceptor sUser32Interceptor;

(nit: naming-wise I think this is named sUser32Intercept elsewhere, in nsWindow, and plugin code, but I think prefer this noun version)

@@ +73,5 @@
> + * InSendMessageEx reports that the calling thread is blocked, that you'll
> + * deadlock your own process. It returns the RPC_E_CANTCALLOUT_ININPUTSYNCCALL
> + * error code. This is not actually true in our case: we are calling into
> + * the multithreaded apartment via ALPC. In this hook, we check to see if the
> + * caller is COM, and if so, we lie to it.

This is sort of terrifying, but hey why not.

Comment 14

[(No longer employed by Mozilla) Aaron Klotz]

https://hg.mozilla.org/integration/mozilla-inbound/rev/59b0e09a9c929c86ec63f043968b83e035a7deda
Bug 1310056: Compatibility hack for mitigating RPC_E_CANTCALLOUT_ININPUTSYNCCALL; r=davidb

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/59b0e09a9c92