All users were logged out of Bugzilla on October 13th, 2018

Do not shut down accessibility if XPCOM cached accessibles are in use.

RESOLVED FIXED in Firefox 53

Status

()

RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: yzen, Assigned: yzen)

Tracking

({access})

unspecified
mozilla53
access
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox53 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

2 years ago
Right now cached xpcom accessibles that are still in use are ignored if the accessibility service reference is not longer used from JS. They should be taken into account and prevent a11y service from shutting down.
(Assignee)

Comment 2

2 years ago
^ scratch that
(Assignee)

Comment 3

2 years ago
Created attachment 8784361 [details] [diff] [review]
1297474 patch
Attachment #8784361 - Flags: review?(surkov.alexander)
Comment on attachment 8784361 [details] [diff] [review]
1297474 patch

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

::: accessible/base/DocManager.cpp
@@ +88,5 @@
> +DocManager::CanBeShutDown()
> +{
> +  for (auto iter = mXPCDocumentCache.Iter(); !iter.Done(); iter.Next()) {
> +    xpcAccessibleDocument* xpcDoc = iter.UserData();
> +    NS_ASSERTION(xpcDoc, "No xpc doc for the object in xpc doc cache!");

MOZ_ASSERT or CRASH

@@ +89,5 @@
> +{
> +  for (auto iter = mXPCDocumentCache.Iter(); !iter.Done(); iter.Next()) {
> +    xpcAccessibleDocument* xpcDoc = iter.UserData();
> +    NS_ASSERTION(xpcDoc, "No xpc doc for the object in xpc doc cache!");
> +    if (xpcDoc && !xpcDoc->CanBeShutDown()) {

can it be really null?

@@ +129,5 @@
>      mXPCDocumentCache.Remove(aDocument);
>    }
>    mDocAccessibleCache.Remove(aDOMDocument);
> +  // Check if the accessibility service can be shut down (if used for XPCOM).
> +  MaybeShutdownAccService();

new line please before the comment

::: accessible/base/DocManager.h
@@ +124,5 @@
> +  /**
> +   * Indicates if cached XPCOM documents are not used via XPCOM and thus
> +   * DocManager would be OK to shut down.
> +   */
> +  bool CanBeShutDown();

there's naming issue, because AccService is inherited from DocManager, and if DocManager can shutdown, then it should mean AccService should be able to shutdown too, but MaybeShutdownAccService says that not. Maybe HasXPCDocuments() or something

::: accessible/base/nsAccessibilityService.h
@@ +310,5 @@
>   * Return accessibility service instance; creating one if necessary.
>   */
>  nsAccessibilityService* GetOrCreateAccService(bool aIsPlatformCaller = true);
>  /**
> + * Shutdown accessibility service if possible.

maybe: 'if needed'

::: accessible/xpcom/xpcAccessibleDocument.cpp
@@ +33,5 @@
>  NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION_INHERITED(xpcAccessibleDocument)
>    NS_INTERFACE_MAP_ENTRY(nsIAccessibleDocument)
>  NS_INTERFACE_MAP_END_INHERITING(xpcAccessibleHyperText)
>  NS_IMPL_ADDREF_INHERITED(xpcAccessibleDocument, xpcAccessibleHyperText)
> +NS_IMETHODIMP_(MozExternalRefCountType) xpcAccessibleDocument::Release(void)

nit: keep return type on own line

@@ +41,5 @@
> +
> +  if (CanBeShutDown()) {
> +    MaybeShutdownAccService();
> +  }
> +

NotifyOfDocumentShutdown seems duping this, because NotifyOfDocumentShutdown will call this Release(). Also can you replace CanBeShutdown() or 'r' check? Having heavy Release() looks worrisome.

Also shutding down the service inside destroying document looks also worrisome, because can you get here during the service shutting down?
Also we could make the cache to keep weak pointers to xpc accessible and make xpc accessible to remove itself from the cache when it gets shutdown. In this case, you probably had a simpler logic: if cache is empty, then you can shutdown.
(Assignee)

Comment 7

2 years ago
Thanks for the review!

I do want to use the weak pointer just wasn't sure how. Do I do it using nsWeakPtr for the type of the value for the caches?

In terms of shutdown stuff, I should be able to just check is a11y service is already shutting down and not call maybeshutdown then?
Flags: needinfo?(surkov.alexander)
(In reply to Yura Zenevich [:yzen] from comment #7)
> Thanks for the review!
> 
> I do want to use the weak pointer just wasn't sure how. Do I do it using
> nsWeakPtr for the type of the value for the caches?

you know it should be even safe to use raw pointers, we should guarantee that the object removes itself from the cache when it's destroyed.

> In terms of shutdown stuff, I should be able to just check is a11y service
> is already shutting down and not call maybeshutdown then?

this might work, but it'd be nicer of course if you avoided loops at all.
Flags: needinfo?(surkov.alexander)
Comment on attachment 8784361 [details] [diff] [review]
1297474 patch

canceling review for now until we sorted out on the approach
Attachment #8784361 - Flags: review?(surkov.alexander)
(Assignee)

Comment 10

2 years ago
(In reply to alexander :surkov from comment #8)
> (In reply to Yura Zenevich [:yzen] from comment #7)
> > Thanks for the review!
> > 
> > I do want to use the weak pointer just wasn't sure how. Do I do it using
> > nsWeakPtr for the type of the value for the caches?
> 
> you know it should be even safe to use raw pointers, we should guarantee
> that the object removes itself from the cache when it's destroyed.
> 
> > In terms of shutdown stuff, I should be able to just check is a11y service
> > is already shutting down and not call maybeshutdown then?
> 
> this might work, but it'd be nicer of course if you avoided loops at all.

here - bug 1316154
(Assignee)

Comment 11

2 years ago
Created attachment 8810444 [details] [diff] [review]
1297474 patch v2

Main difference - keep a strong ref to an xpcDocument until it's cache is empty. This ensures that even if xpcAccDoc reference is removed in JS we do not clear DocManager's cache.
Attachment #8784361 - Attachment is obsolete: true
Attachment #8810444 - Flags: review?(surkov.alexander)
Comment on attachment 8810444 [details] [diff] [review]
1297474 patch v2

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

r=me with questions answered/addressed (especially xpcAccessibleDocument.h, btw, it seems you dont' have test for this scenario)

::: accessible/base/DocManager.cpp
@@ +87,5 @@
> +bool
> +DocManager::HasXPCDocuments()
> +{
> +  return mXPCDocumentCache.Count() > 0 ||
> +         (sRemoteXPCDocumentCache && sRemoteXPCDocumentCache->Count() > 0);

make it inline?

::: accessible/tests/browser/browser.ini
@@ +3,5 @@
>    head.js
>    shared-head.js
> +[browser_shutdown_acc_reference.js]
> +[browser_shutdown_doc_acc_reference.js]
> +[browser_shutdown_multi_acc_reference.js]

I might miss it, you don't testing for proxy stuff?

::: accessible/tests/browser/browser_shutdown_acc_reference.js
@@ +15,5 @@
> +
> +  // Accessible object reference will live longer than the scope of this
> +  // function.
> +  let acc = yield new Promise(resolve => {
> +    let intervalId = setInterval(() => {

curious, why you prefer interval stuff over events

::: accessible/xpcom/xpcAccessibleDocument.h
@@ +80,4 @@
>      mCache.Remove(aAccessible);
> +    if (mCache.Count() == 0) {
> +      GetAccService()->RemoveFromXPCDocumentCache(
> +        mIntl.AsAccessible()->AsDoc());

could it be that the document itself is kept by a script?

::: accessible/xpcom/xpcAccessibleGeneric.cpp
@@ +19,5 @@
>  NS_INTERFACE_MAP_END
> +NS_IMPL_ADDREF(xpcAccessibleGeneric)
> +NS_IMPL_RELEASE(xpcAccessibleGeneric)
> +
> +xpcAccessibleGeneric::~xpcAccessibleGeneric() {

nit: { on a new line

@@ +24,5 @@
> +  if (mIntl.IsNull()) {
> +    return;
> +  }
> +
> +  xpcAccessibleDocument* xpcDoc;

nit: = nullptr;

@@ +27,5 @@
> +
> +  xpcAccessibleDocument* xpcDoc;
> +  if (mIntl.IsAccessible()) {
> +    Accessible* acc = mIntl.AsAccessible();
> +    if (acc->IsDoc()) {

nit: I think I would prefer if (!acc->IsDoc()) {}

::: accessible/xpcom/xpcAccessibleGeneric.h
@@ +38,5 @@
>        mSupportedIfaces |= eHyperLink;
>    }
>    xpcAccessibleGeneric(ProxyAccessible* aProxy, uint8_t aInterfaces) :
>      mIntl(aProxy), mSupportedIfaces(aInterfaces) {}
> +  NS_DECL_ISUPPORTS

could you get thumb up from smaug on nsISupports changes?
Attachment #8810444 - Flags: review?(surkov.alexander) → review+

Comment 19

2 years ago
Pushed by yura.zenevich@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/48ac718556a0
shutdown accessibility service only if there are no references to accessibles in JS. r=surkov
Backed out for crash in Marionette test test_switch_remote_frame.py TestSwitchRemoteFrame.test_remote_frame_revisit and failing T-e10s(o) / a11yr:

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

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=48ac718556a01201c327e022ed83825c2dbe5f56
Failure log Marionette: https://treeherder.mozilla.org/logviewer.html#?job_id=39500431&repo=mozilla-inbound

22:02:52     INFO - TEST-START | test_switch_remote_frame.py TestSwitchRemoteFrame.test_remote_frame_revisit
22:02:54     INFO - mozcrash Downloading symbols from: https://queue.taskcluster.net/v1/task/ZbI4zcQoSwGPVqEr5KbK0Q/artifacts/public/build/firefox-53.0a1.en-US.linux-x86_64.crashreporter-symbols.zip
22:03:16     INFO - mozcrash Copy/paste: /builds/slave/test/build/linux64-minidump_stackwalk /tmp/tmpv46fF3.mozrunner/minidumps/330e67f1-3abe-4e1c-20160d23-365c1928.dmp /tmp/tmp7v5Tps
22:03:30     INFO - mozcrash Saved minidump as /builds/slave/test/build/blobber_upload_dir/330e67f1-3abe-4e1c-20160d23-365c1928.dmp
22:03:30     INFO - mozcrash Saved app info as /builds/slave/test/build/blobber_upload_dir/330e67f1-3abe-4e1c-20160d23-365c1928.extra
22:03:30     INFO - PROCESS-CRASH | test_switch_remote_frame.py TestSwitchRemoteFrame.test_remote_frame_revisit | application crashed [@ mozilla::a11y::DocManager::RemoveFromRemoteXPCDocumentCache]
22:03:30     INFO - Crash dump filename: /tmp/tmpv46fF3.mozrunner/minidumps/330e67f1-3abe-4e1c-20160d23-365c1928.dmp
22:03:30     INFO - Operating system: Linux
22:03:30     INFO -                   0.0.0 Linux 3.2.0-76-generic #111-Ubuntu SMP Tue Jan 13 22:16:09 UTC 2015 x86_64
22:03:30     INFO - CPU: amd64
22:03:30     INFO -      family 6 model 62 stepping 4
22:03:30     INFO -      1 CPU
22:03:30     INFO - 
22:03:30     INFO - GPU: UNKNOWN
22:03:30     INFO - 
22:03:30     INFO - Crash reason:  SIGSEGV
22:03:30     INFO - Crash address: 0x10
22:03:30     INFO - Process uptime: not available
22:03:30     INFO - 
22:03:30     INFO - Thread 0 (crashed)
22:03:30     INFO -  0  libxul.so!mozilla::a11y::DocManager::RemoveFromRemoteXPCDocumentCache [DocManager.cpp:48ac718556a0 : 129 + 0x0]
22:03:30     INFO -     rax = 0x0000000000000000   rdx = 0x00007f46d9a00048
22:03:30     INFO -     rcx = 0x000000000000001d   rbx = 0x00007f468f6f4d40
22:03:30     INFO -     rsi = 0x00007f468f6f4d40   rdi = 0x0000000000000000
22:03:30     INFO -     rbp = 0x00007fff95bf0130   rsp = 0x00007fff95bf0120
22:03:30     INFO -      r8 = 0x00007f469a800000    r9 = 0x000000000000560d
22:03:30     INFO -     r10 = 0x00007f468db9a080   r11 = 0x00007f4699178860
22:03:30     INFO -     r12 = 0x00007fff95bf0140   r13 = 0x0000000000000000
22:03:30     INFO -     r14 = 0x0000000000000006   r15 = 0x0000000000000000
22:03:30     INFO -     rip = 0x00007f46d0365c65
22:03:30     INFO -     Found by: given as instruction pointer in context
22:03:30     INFO -  1  libxul.so!mozilla::a11y::DocAccessibleParent::Destroy [DocAccessibleParent.cpp:48ac718556a0 : 433 + 0x8]
22:03:30     INFO -     rbx = 0x00007f468f6f4d40   rbp = 0x00007fff95bf0190
22:03:30     INFO -     rsp = 0x00007fff95bf0140   r12 = 0x00007fff95bf0140
22:03:30     INFO -     r13 = 0x0000000000000000   r14 = 0x0000000000000006
22:03:30     INFO -     r15 = 0x0000000000000000   rip = 0x00007f46d0394134
22:03:30     INFO -     Found by: call frame info
22:03:30     INFO -  2  libxul.so!mozilla::a11y::DocAccessibleParent::RecvShutdown [DocAccessibleParent.cpp:48ac718556a0 : 404 + 0x5]
22:03:30     INFO -     rbx = 0x00007f468f6f4d40   rbp = 0x00007fff95bf01b0
22:03:30     INFO -     rsp = 0x00007fff95bf01a0   r12 = 0x00007fff95bf0228
22:03:30     INFO -     r13 = 0x0000000000000001   r14 = 0x0000000000000006
22:03:30     INFO -     r15 = 0x0000000000000000   rip = 0x00007f46d0394266
22:03:30     INFO -     Found by: call frame info
22:03:30     INFO -  3  libxul.so!mozilla::a11y::PDocAccessibleParent::OnMessageReceived [PDocAccessibleParent.cpp:48ac718556a0 : 6610 + 0x6]
22:03:30     INFO -     rbx = 0x0000000000000000   rbp = 0x00007fff95bf0270
22:03:30     INFO -     rsp = 0x00007fff95bf01c0   r12 = 0x00007fff95bf0228
22:03:30     INFO -     r13 = 0x0000000000000001   r14 = 0x0000000000000006
22:03:30     INFO -     r15 = 0x0000000000000000   rip = 0x00007f46cf007d17
22:03:30     INFO -     Found by: call frame info
22:03:30     INFO -  4  libxul.so!mozilla::dom::PContentParent::OnMessageReceived [PContentParent.cpp:48ac718556a0 : 3069 + 0xc]
22:03:30     INFO -     rbx = 0x00007f46a39a8000   rbp = 0x00007fff95bf0510
22:03:30     INFO -     rsp = 0x00007fff95bf0280   r12 = 0x00007f468db83d00
22:03:30     INFO -     r13 = 0x0000000000000001   r14 = 0x0000000000000006
22:03:30     INFO -     r15 = 0x0000000000000000   rip = 0x00007f46cefe7e5e
22:03:30     INFO -     Found by: call frame info
22:03:30     INFO -  5  libxul.so!mozilla::ipc::MessageChannel::DispatchAsyncMessage [MessageChannel.cpp:48ac718556a0 : 1743 + 0x6]
22:03:30     INFO -     rbx = 0x00007f46a39a80a8   rbp = 0x00007fff95bf0550
22:03:30     INFO -     rsp = 0x00007fff95bf0520   r12 = 0x00007f468db83d00
22:03:30     INFO -     r13 = 0x0000000000000001   r14 = 0x00007fff95bf0500
22:03:30     INFO -     r15 = 0x0000000000000000   rip = 0x00007f46cee2172d
22:03:30     INFO -     Found by: call frame info
22:03:30     INFO -  6  libxul.so!mozilla::ipc::MessageChannel::DispatchMessage [MessageChannel.cpp:48ac718556a0 : 1681 + 0xb]
22:03:30     INFO -     rbx = 0x00007f468db83d00   rbp = 0x00007fff95bf0600
22:03:30     INFO -     rsp = 0x00007fff95bf0560   r12 = 0x00007f46a39a80a8
22:03:30     INFO -     r13 = 0x00007fff95bf0578   r14 = 0x00007fff95bf05a8
22:03:30     INFO -     r15 = 0x00007fff95bf0580   rip = 0x00007f46cee289b2
22:03:30     INFO -     Found by: call frame info
22:03:30     INFO -  7  libxul.so!mozilla::ipc::MessageChannel::RunMessage [MessageChannel.cpp:48ac718556a0 : 1572 + 0xb]
22:03:30     INFO -     rbx = 0x00007f46a39a80a8   rbp = 0x00007fff95bf0640
22:03:30     INFO -     rsp = 0x00007fff95bf0610   r12 = 0x00007f468db83d00
22:03:30     INFO -     r13 = 0x00007f468db83cc0   r14 = 0x00007fff95bf06a8
22:03:30     INFO -     r15 = 0x00007fff95bf072f   rip = 0x00007f46cee2aac3
22:03:30     INFO -     Found by: call frame info
22:03:30     INFO -  8  libxul.so!mozilla::ipc::MessageChannel::MessageTask::Run [MessageChannel.cpp:48ac718556a0 : 1597 + 0xc]
22:03:30     INFO -     rbx = 0x00007f468db83cc0   rbp = 0x00007fff95bf0670
22:03:30     INFO -     rsp = 0x00007fff95bf0650   r12 = 0x00007fff95bf06a0
22:03:30     INFO -     r13 = 0x0000000000000000   r14 = 0x00007fff95bf06a8
22:03:30     INFO -     r15 = 0x00007fff95bf072f   rip = 0x00007f46cee2ac19
22:03:30     INFO -     Found by: call frame info
22:03:30     INFO -  9  libxul.so!nsThread::ProcessNextEvent [nsThread.cpp:48ac718556a0 : 1213 + 0x6]
22:03:30     INFO -     rbx = 0x00007f46d9904680   rbp = 0x00007fff95bf0710
22:03:30     INFO -     rsp = 0x00007fff95bf0680   r12 = 0x00007fff95bf06a0
22:03:30     INFO -     r13 = 0x0000000000000000   r14 = 0x00007fff95bf06a8
22:03:30     INFO -     r15 = 0x00007fff95bf072f   rip = 0x00007f46ceb40aa0
22:03:30     INFO -     Found by: call frame info
22:03:30     INFO - 10  libxul.so!NS_ProcessNextEvent [nsThreadUtils.cpp:48ac718556a0 : 361 + 0xd]
22:03:30     INFO -     rbx = 0x0000000000000000   rbp = 0x00007fff95bf0740
22:03:30     INFO -     rsp = 0x00007fff95bf0720   r12 = 0x00007f46d99f2bc0
22:03:30     INFO -     r13 = 0x00007f46d9904680   r14 = 0x00007fff95bf0801
22:03:30     INFO -     r15 = 0x00007f46d99fbea0   rip = 0x00007f46ceb5aa7c
22:03:30     INFO -     Found by: call frame info
22:03:30     INFO - 11  libxul.so!mozilla::ipc::MessagePump::Run [MessagePump.cpp:48ac718556a0 : 96 + 0xa]
22:03:30     INFO -     rbx = 0x00007f46d99fbe80   rbp = 0x00007fff95bf07a0
22:03:30     INFO -     rsp = 0x00007fff95bf0750   r12 = 0x00007f46d99f2bc0
22:03:30     INFO -     r13 = 0x00007f46d9904680   r14 = 0x00007fff95bf0801
22:03:30     INFO -     r15 = 0x00007f46d99fbea0   rip = 0x00007f46cee1d9f4
22:03:30     INFO -     Found by: call frame info
22:03:30     INFO - 12  libxul.so!MessageLoop::Run [message_loop.cc:48ac718556a0 : 225 + 0x8]
22:03:30     INFO -     rbx = 0x00007f46d99f2bc0   rbp = 0x00007fff95bf07e0
22:03:30     INFO -     rsp = 0x00007fff95bf07b0   r12 = 0x00007f46d9904680
22:03:30     INFO -     r13 = 0x00007fff95bf08b0   r14 = 0x00007fff95bf08a0
22:03:30     INFO -     r15 = 0x00007fff95bf0b81   rip = 0x00007f46cee07f6e
22:03:30     INFO -     Found by: call frame info
22:03:30     INFO - 13  libxul.so!nsBaseAppShell::Run [nsBaseAppShell.cpp:48ac718556a0 : 156 + 0xd]
22:03:30     INFO -     rbx = 0x00007f46bf392340   rbp = 0x00007fff95bf0800
22:03:30     INFO -     rsp = 0x00007fff95bf07f0   r12 = 0x00007f46d9904680
22:03:30     INFO -     r13 = 0x00007fff95bf08b0   r14 = 0x00007fff95bf08a0
22:03:30     INFO -     r15 = 0x00007fff95bf0b81   rip = 0x00007f46cfea9c9b
22:03:30     INFO -     Found by: call frame info
22:03:30     INFO - 14  libxul.so!nsAppStartup::Run [nsAppStartup.cpp:48ac718556a0 : 283 + 0x6]
22:03:30     INFO -     rbx = 0x00007f46bf388060   rbp = 0x00007fff95bf0820
22:03:30     INFO -     rsp = 0x00007fff95bf0810   r12 = 0x0000000000000007
22:03:30     INFO -     r13 = 0x00007fff95bf08b0   r14 = 0x00007fff95bf08a0
22:03:30     INFO -     r15 = 0x00007fff95bf0b81   rip = 0x00007f46d04d487f
22:03:30     INFO -     Found by: call frame info
22:03:30     INFO - 15  libxul.so!XREMain::XRE_mainRun [nsAppRunner.cpp:48ac718556a0 : 4467 + 0x6]
22:03:30     INFO -     rbx = 0x00007fff95bf08e0   rbp = 0x00007fff95bf0970
22:03:30     INFO -     rsp = 0x00007fff95bf0830   r12 = 0x0000000000000007
22:03:30     INFO -     r13 = 0x00007fff95bf08b0   r14 = 0x00007fff95bf08a0
22:03:30     INFO -     r15 = 0x00007fff95bf0b81   rip = 0x00007f46d0523f02
22:03:30     INFO -     Found by: call frame info
22:03:30     INFO - 16  libxul.so!XREMain::XRE_main [nsAppRunner.cpp:48ac718556a0 : 4600 + 0x5]
22:03:30     INFO -     rbx = 0x00007fff95bf09e8   rbp = 0x00007fff95bf09c0
22:03:30     INFO -     rsp = 0x00007fff95bf0980   r12 = 0x0000000000000000
22:03:30     INFO -     r13 = 0x00007fff95bf0990   r14 = 0x0000000000000000
22:03:30     INFO -     r15 = 0x00007f46d9942540   rip = 0x00007f46d05241c2
22:03:30     INFO -     Found by: call frame info
22:03:30     INFO - 17  libxul.so!XRE_main [nsAppRunner.cpp:48ac718556a0 : 4691 + 0x5]
22:03:30     INFO -     rbx = 0x00007fff95bf09e8   rbp = 0x00007fff95bf0bb0
22:03:30     INFO -     rsp = 0x00007fff95bf09d0   r12 = 0x0000000000000005
22:03:30     INFO -     r13 = 0x00007fff95bf1d78   r14 = 0x00007fff95bf0c20
22:03:30     INFO -     r15 = 0x00007f46d9942540   rip = 0x00007f46d0524456
22:03:30     INFO -     Found by: call frame info
22:03:30     INFO - 18  firefox!do_main [nsBrowserApp.cpp:48ac718556a0 : 328 + 0x6]
22:03:30     INFO -     rbx = 0x00007fff95bf1d78   rbp = 0x00007fff95bf1c50
22:03:30     INFO -     rsp = 0x00007fff95bf0bc0   r12 = 0x0000000000000005
22:03:30     INFO -     r13 = 0x00007fff95bf0c20   r14 = 0x00007f46d9942780
22:03:30     INFO -     r15 = 0x00007f46d9942540   rip = 0x00000000004054cd
22:03:30     INFO -     Found by: call frame info
22:03:30     INFO - 19  firefox!main [nsBrowserApp.cpp:48ac718556a0 : 461 + 0x12]
22:03:30     INFO -     rbx = 0x00007fff95bf1d78   rbp = 0x00007fff95bf1c90
22:03:30     INFO -     rsp = 0x00007fff95bf1c60   r12 = 0x0000000000000005
22:03:30     INFO -     r13 = 0x0000065ca1f69506   r14 = 0x00007fff95bf1da8
22:03:30     INFO -     r15 = 0x0000000000000000   rip = 0x0000000000404a2c
22:03:30     INFO -     Found by: call frame info
22:03:30     INFO - 20  libc-2.15.so + 0x2176d
22:03:30     INFO -     rbx = 0x0000000000000000   rbp = 0x0000000000000000
22:03:30     INFO -     rsp = 0x00007fff95bf1ca0   r12 = 0x0000000000404bd0
22:03:30     INFO -     r13 = 0x00007fff95bf1d70   r14 = 0x0000000000000000
22:03:30     INFO -     r15 = 0x0000000000000000   rip = 0x00007f46d9b3776d
22:03:30     INFO -     Found by: call frame info
22:03:30     INFO - 21  firefox!init [replace_malloc.c:48ac718556a0 : 133 + 0x2]
22:03:30     INFO -     rsp = 0x00007fff95bf1cc0   rip = 0x00000000004049d7
22:03:30     INFO -     Found by: stack scanning

Failure log T-e10s(o): https://treeherder.mozilla.org/logviewer.html#?job_id=39500379&repo=mozilla-inbound
Flags: needinfo?(yzenevich)

Comment 21

2 years ago
Pushed by yura.zenevich@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e79129082f4a
shutdown accessibility service only if there are no references to accessibles in JS. r=surkov
(Assignee)

Comment 22

2 years ago
^ should resolve the crash based on the try run below:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=580867f7c10e661667bc19da4429ec1ec6214748
Flags: needinfo?(yzenevich)
sorry had to back this out again for crashes like https://treeherder.mozilla.org/logviewer.html#?job_id=39536230&repo=mozilla-inbound
Flags: needinfo?(yzenevich)

Comment 24

2 years ago
Backout by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c7be1246b301
Backed out changeset e79129082f4a for crashes in m-oth windows tests
(Assignee)

Comment 25

2 years ago
Created attachment 8812818 [details] [diff] [review]
xpc appliaction accessible patch
Flags: needinfo?(yzenevich)
Attachment #8812818 - Flags: review?(surkov.alexander)
Comment on attachment 8812818 [details] [diff] [review]
xpc appliaction accessible patch

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

::: accessible/xpcom/xpcAccessibleApplication.h
@@ +33,5 @@
>  protected:
> +  virtual ~xpcAccessibleApplication()
> +  {
> +    Shutdown();
> +  }

a nicer approach would be fixing Accessible::LastRelease() by replacing mDoc on eDefunct check
Attachment #8812818 - Flags: review?(surkov.alexander) → review+
Comment on attachment 8812818 [details] [diff] [review]
xpc appliaction accessible patch

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

::: accessible/xpcom/xpcAccessibleApplication.h
@@ +33,5 @@
>  protected:
> +  virtual ~xpcAccessibleApplication()
> +  {
> +    Shutdown();
> +  }

ignore it, this is xpcom version

Comment 29

2 years ago
Pushed by yura.zenevich@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a9f041fdef88
clean up internal accessible when xpcAccessibleApplication is being destroyed. r=surkov
https://hg.mozilla.org/integration/mozilla-inbound/rev/ddc9b2697068
shutdown accessibility service only if there are no references to accessibles in JS. r=surkov

Comment 30

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a9f041fdef88
https://hg.mozilla.org/mozilla-central/rev/ddc9b2697068
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Depends on: 1330765
You need to log in before you can comment on or make changes to this bug.