Closed Bug 1650590 Opened 4 years ago Closed 4 years ago

Firefox crashes when JAWS is running on a specific webpage

Categories

(Core :: Disability Access APIs, defect, P1)

78 Branch
Desktop
Windows
defect

Tracking

()

VERIFIED FIXED
mozilla80
Tracking Status
firefox-esr68 --- unaffected
firefox-esr78 79+ fixed
firefox77 --- unaffected
firefox78 + wontfix
firefox79 + verified
firefox80 + verified

People

(Reporter: lukasz.golonka, Assigned: Jamie)

References

(Regression)

Details

(Keywords: crash, regression)

Crash Data

Attachments

(2 files)

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:80.0) Gecko/20100101 Firefox/80.0

Steps to reproduce:

With JAWS 2020 running open https://raywoodcockslatest.wordpress.com/ and wait a few seconds after page loads.

Actual results:

The message "your tab just crashed" appears. The latest crash report is available at:
https://crash-stats.mozilla.org/report/index/2ebb5ee4-5608-4b21-a0af-940ee0200704#tab-bugzilla

Expected results:

No crash.

Component: Untriaged → Disability Access APIs
Product: Firefox → Core
See Also: → 1650669

Marco: Would you be able to confirm this crash?

Severity: -- → S2
Flags: needinfo?(mzehe)

Unfortunately I am not. I have been trying all Friday, after receiving reports from Léonie Watson and Dónal Fitzpatrick on Twitter, as well as one other report via e-mail. Neither of these three were able to give me any crash reports, though, there weren't any created for these tab crashes. The above crash, however, points to some C Runtime library stuff, which may or may not have to do something with our AccessibleHandler code. However, I tried reproducing this on Friday, with both 78 and Nightly, and could not. Neither could Asa. Redirecting NI to Jamie.

Flags: needinfo?(mzehe) → needinfo?(jteh)

And I suspect bug 1650669 to be a duplicate of this one.

OK, after another critical piece of information, I am finally able to reproduce the crash.

  1. With JAWS 2020.2006.12, which is the latest update to JAWS 2020, go to https://github.com/nvaccess/nvda.
  2. Above the Latest Commit heading, find the Code collapsed button.
  3. Press the button using Space.

Result: Tab crashes, but no crash report is generated.

  1. Select to restore the tab.
  2. Go to about:config and in the search field, type the word handler.
  3. This will bring up the accessibility.AccessibleHandler preference, which defaults to True. Toggle it to false.
  4. Restart Firefox. Note that things will be considerably slower now.
  5. Repeat steps 1 through 3.

Result: No crash.

The piece that is causing this is, therefore, the accessible handler. And because of its COM stuff, it probably also is the reason why there are hardly ever any crash reports. In fact, the above crash report is the only one I know that was ever generated from such a crash.

This affects all versions since 78, 78-ESR, and includes 79 beta and 80 Nightly.

Status: UNCONFIRMED → NEW
Has STR: --- → yes
Ever confirmed: true
Keywords: crash, regression
OS: Unspecified → Windows
Priority: -- → P1
Hardware: Unspecified → Desktop
Version: 80 Branch → 78 Branch

Through bisecting of affected AccessibleHandler work nightlies, determined that bug 1640553 is the offending one, and not even the followup bug 1642141 fixed it. This is a different crash.

Has Regression Range: --- → yes
Regressed by: 1640553

I set Windows to create a mini dump, since I learned that we are missing quite a number of crashes. This appears to be an EXCEPTION_HEAP_CORRUPTION crash. Got instructions and a script to get the information out of that mini dump, and here it is. This is the same info you'd see on a public crash report if a crash was submitted. This was done with a current, 2020-07-05, Nightly build.

I have to have a sighted person with some technical ability come over to fix this so I can do my billing. What should I tell them? Go back to JAWS 18? Does Firefox 78 crash with JAWS 18? If it does, how do I turn off automatic update in Firefox so I can go back to 77, which works with JAWS and not have the darn thing just update itself again?

Here is a related issue of an ARIA Practices example for a date picker combobox crashing when using NVDA or JAWS on windows:
https://github.com/w3c/aria-practices/issues/1440

Jon, actually that one is different, because it also reproduces with NVDA. It also generates crash reports. We are tracking this in bug 1650348. However, this may be related to this bug, only exposed a little differently.

DynamicIA2Data can be built to be transmitted in two different ways:

  1. As part of the payload included in the stream when an accessible is marshaled; or
  2. As an out parameter returned by IGeckoBackChannel::Refresh().

DynamicIA2Data includes arrays for row/column header ids.
Normally, such arrays would be allocated by CoTaskMemFree and freed by CoTaskMemAlloc.
However, in the first case, the struct is actually marshaled by RPC encoding functions, not by COM itself.
This means we must use midl_user_allocate/free, lest we crash.
We previously used midl_user_allocate/free for the second case as well.
Unfortunately, it turns out that this too causes crashes.

To fix this, we now use different memory allocation functions depending on how the struct is transmitted.

This patch also cleans up the old DynamicIA2Data in the client before calling IGeckoBackChannel::Refresh.
Previously, we didn't do this, which would have resulted in a leak.

Assignee: nobody → jteh
Status: NEW → ASSIGNED

(In reply to James Teh [:Jamie] from comment #11)

This patch also cleans up the old DynamicIA2Data in the client before calling IGeckoBackChannel::Refresh.
Previously, we didn't do this, which would have resulted in a leak.

I'm wondering whether this bit might help with bug 1572915 (AKA RPC hang). I'm really not certain, but my suspicion is that any COM or RPC related leak might make that bug more likely.

Blocks: 1572915
Flags: needinfo?(jteh)
Pushed by jteh@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b817b26b2633
A11y COM handler: Use CoTaskMemAlloc/Free for row/column header ids arrays when returned by IGeckoBackChannel::Refresh. r=MarcoZ
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla80

Comment on attachment 9162367 [details]
Bug 1650590: A11y COM handler: Use CoTaskMemAlloc/Free for row/column header ids arrays when returned by IGeckoBackChannel::Refresh.

Beta/Release Uplift Approval Request

  • User impact if declined: Frequent crashes for JAWS screen reader users (and occasional crashes for NVDA screen reader users) on pages containing dynamic tables.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Medium
  • Why is the change risky/not risky? (and alternatives if risky): Some risk of crashes given this hasn't baked on Nightly, but I think this is far outweighed by the frequency and severity of the current crashes.
  • String changes made/needed:
Attachment #9162367 - Flags: approval-mozilla-beta?

Comment on attachment 9162367 [details]
Bug 1650590: A11y COM handler: Use CoTaskMemAlloc/Free for row/column header ids arrays when returned by IGeckoBackChannel::Refresh.

Approved for 79.0b6 so we can get verification as soon as possible. It would be great if we could get some affected users to try this out when 79.0b6 is on the Beta channel in ~8hr, give or take.

Attachment #9162367 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

I will be happy to test as I can reproduce this behavior very consistently.

Blocks: 1650348

Verified fixed in 80.0a1 (20200709213735).

Status: RESOLVED → VERIFIED

Verified that this issue is no longer reproducible in 79.0b6 as described in comment #4.

Comment on attachment 9162367 [details]
Bug 1650590: A11y COM handler: Use CoTaskMemAlloc/Free for row/column header ids arrays when returned by IGeckoBackChannel::Refresh.

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: Frequent crashes impacting daily usage of screen reader users.
  • User impact if declined: Frequent crashes for JAWS screen reader users (and occasional crashes for NVDA screen reader users) on pages containing dynamic tables.
  • Fix Landed on Version: 79
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Verified on both nightly and beta, both internally and by users outside of Mozilla. Cause and fix is well understood.
  • String or UUID changes made by this patch:
Attachment #9162367 - Flags: approval-mozilla-esr78?

Comment on attachment 9162367 [details]
Bug 1650590: A11y COM handler: Use CoTaskMemAlloc/Free for row/column header ids arrays when returned by IGeckoBackChannel::Refresh.

Approved for 78.1esr.

Attachment #9162367 - Flags: approval-mozilla-esr78? → approval-mozilla-esr78+

Fx79 is in RC, so wontfix for 78.

Crash Signature: [@ RtlSizeHeap | NS_FaultTolerantHeap::APIHook_RtlFreeHeap]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: