Closed Bug 1642141 Opened 5 years ago Closed 5 years ago

Crash in [@ mozilla::a11y::AccessibleHandlerControl::CacheAccessible]

Categories

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

78 Branch
x86_64
Windows 10
defect

Tracking

()

VERIFIED FIXED
mozilla79
Tracking Status
firefox-esr68 --- unaffected
firefox76 --- unaffected
firefox77 --- unaffected
firefox78 --- verified
firefox79 --- verified

People

(Reporter: MarcoZ, Assigned: Jamie)

References

(Regression)

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

This bug is for crash report bp-f28e079c-53f2-4544-bfd1-a4e5e0200530.

Top 10 frames of crashing thread:

0 accessiblehandler.dll mozilla::a11y::AccessibleHandlerControl::CacheAccessible accessible/ipc/win/handler/AccessibleHandlerControl.cpp:178
1 accessiblehandler.dll mozilla::a11y::AccessibleHandler::ReadHandlerPayload accessible/ipc/win/handler/AccessibleHandler.cpp:507
2 accessiblehandler.dll mozilla::mscom::Handler::UnmarshalInterface ipc/mscom/oop/Handler.cpp:255
3 combase.dll CoUnmarshalInterface onecore\com\combase\dcomrem\coapi.cxx:1951
4 combase.dll Ndr64ExtInterfacePointerUnmarshall onecore\com\combase\ndr\ndrole64\oleaux64.cxx:463
5 rpcrt4.dll void Ndr64pPointerUnmarshallInternal 
6 rpcrt4.dll void Ndr64pPointerUnmarshallInternal 
7 rpcrt4.dll Ndr64pClientUnMarshal 
8 rpcrt4.dll NdrpClientCall3 
9 combase.dll ObjectStublessClient onecore\com\combase\ndr\ndrole\amd64\stblsclt.cxx:292

STR, with the NVDA screen reader:

  1. I was on this page.
  2. Navigated to the first table on the page.
  3. Used CTRL+Alt+DownArrow to walk down the first column.
    • Result: At row 3 or so, I got this crash.

Looks like the new caching code in the handler to me.

Wow. That's nasty.

  1. An event invalidates the cache.
  2. That clears the accessible cache.
  3. That calls Release on a remote accessible.
  4. While we're waiting for that to happen, COM re-enters and executes another client call (in this case, from UIA, but it could be anything).
  5. That fetches a new table header accessible.
  6. We try to add it to the accessible cache.
  7. Crash.

I'm guessing unordered_map doesn't like us messing with the map while it's being cleared. Hopefully, we can fix this by swapping the map into a temporary map and then clearing the temporary map.

Regressed by: 1640553
No longer regressed by: 1638238
Has Regression Range: --- → yes
Assignee: nobody → jteh
Status: NEW → ASSIGNED
Pushed by jteh@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f262a231f046 When clearing the AccessibleHandlerControl Accessible cache, swap it into a temporary map first to avoid re-entry into the main map while clearing. r=MarcoZ
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla79

Please request beta uplift also.

Flags: needinfo?(jteh)

Marco, I never saw this myself. Could you verify it's fixed in Nightly for you? Thanks.

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

Just confirmed that it is fixed.

Flags: needinfo?(mzehe)

Comment on attachment 9153041 [details]
Bug 1642141: When clearing the AccessibleHandlerControl Accessible cache, swap it into a temporary map first to avoid re-entry into the main map while clearing.

Beta/Release Uplift Approval Request

  • User impact if declined: Crashes.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Simply moves data before destroying it to prevent re-entry.
  • String changes made/needed:
Attachment #9153041 - Flags: approval-mozilla-beta?

Comment on attachment 9153041 [details]
Bug 1642141: When clearing the AccessibleHandlerControl Accessible cache, swap it into a temporary map first to avoid re-entry into the main map while clearing.

a11y regression fix, approved for 78.0b3

Attachment #9153041 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

Confirming that this issue no longer reproduces on Windows 10 x64 using NVDA 2020.1 with Firefox 78.0b4 and latest Nightly.

Also checked Socorro and confirmed that no new crashes occured after this fix landed.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: