Closed Bug 1447232 Opened 6 years ago Closed 4 years ago

[tier 2][test-verify][perma-orange] layout/base/tests/test_event_target_iframe_oop.html | application crashed [@ ParentImpl::AssertIsOnBackgroundThread]

Categories

(Core :: Layout, defect, P2)

defect

Tracking

()

RESOLVED INACTIVE

People

(Reporter: intermittent-bug-filer, Unassigned)

References

Details

(Keywords: crash)

Crash Data

Summary: Intermittent layout/base/tests/test_event_target_iframe_oop.html | application crashed [@ ParentImpl::AssertIsOnBackgroundThread] → [perma-orange] layout/base/tests/test_event_target_iframe_oop.html | application crashed [@ ParentImpl::AssertIsOnBackgroundThread]
If this is a perma-orange in a tier-1 test on mozilla-inbound, shouldn't it have led to the changeset that caused it being backed out?

Then again, given that the first occurrence was 04:52 UTC and the last was 10:59 UTC (almost 15 hours ago), it seems like it probably was backed out.
Status: NEW → RESOLVED
Closed: 6 years ago
Priority: P5 → P1
Resolution: --- → WORKSFORME
Oh, this is a tier-2 test-verify job.
Status: RESOLVED → REOPENED
Priority: P1 → P2
Resolution: WORKSFORME → ---
Summary: [perma-orange] layout/base/tests/test_event_target_iframe_oop.html | application crashed [@ ParentImpl::AssertIsOnBackgroundThread] → [tier 2][perma-orange] layout/base/tests/test_event_target_iframe_oop.html | application crashed [@ ParentImpl::AssertIsOnBackgroundThread]
It's not clear to me if this happened just because bz *touched* the test in bug 1446711 (is this one of the test runs that only happens when somebody touches a test?) or if it's actually a result of the changes.
Blocks: 1446711
Summary: [tier 2][perma-orange] layout/base/tests/test_event_target_iframe_oop.html | application crashed [@ ParentImpl::AssertIsOnBackgroundThread] → [tier 2][test-verify][perma-orange] layout/base/tests/test_event_target_iframe_oop.html | application crashed [@ ParentImpl::AssertIsOnBackgroundThread]
Yeah, this test got run because I touched it.

The crash stack here is:

  ParentImpl::AssertIsOnBackgroundThread
  mozilla::dom::StorageDBParent::~StorageDBParent
  mozilla::dom::StorageDBParent::Release
  RefPtr<mozilla::dom::StorageDBParent>::~RefPtr
  CheckLowDiskSpaceRunnable::~CheckLowDiskSpaceRunnable
  ...
  nsThread::ProcessNextEvent

and I expect we're failing that IsOnBackgroundThread assert.

The setup here is that StorageDBParent::Init is called on the background thread, creates a CheckLowDiskSpaceRunnable and dispatches it to the main thread.

When that runnable runs on the main thread, it redispatches itself back to the thread it came from.

When it runs back on that thread, it drops the ref to the StorageDBParent.

This is all fine, but if the thread the runnable came from has been shut down, dispatch back to there will fail, the runnable will get destroyed on the main thread, and destroy the StorageDBParent on the main thread too.

Jan, Andrew, can that happen here?
Flags: needinfo?(jvarga)
Flags: needinfo?(bugmail)
(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #6)
> This is all fine, but if the thread the runnable came from has been shut
> down, dispatch back to there will fail, the runnable will get destroyed on
> the main thread, and destroy the StorageDBParent on the main thread too.
> 
> Jan, Andrew, can that happen here?

Yes, that seems to be happening.

Since:
- I don't believe the disk space watcher is actually implemented anywhere in-tree with the removal of hal/gonk.  (That is, we have a bunch of stubs, but nothing that will actually invoke DiskSpaceWatcher::UpdateState and cause useful things to happen.)
- We're imminently going to land a complete overhaul of LocalStorage once I do the reviews, and I'll make sure we don't repeat this life-cycle glitch there.

Maybe we should just comment out the creation and dispatching of the runnable at https://searchfox.org/mozilla-central/rev/b29daa46443b30612415c35be0a3c9c13b9dc5f6/dom/storage/StorageIPC.cpp#569

Aside: The test deals with event fluffing, what a b2g blast from the past!
Flags: needinfo?(bugmail)
Yes, that code is going away soon. 

> Maybe we should just comment out the creation and dispatching of the runnable at https://searchfox.org/mozilla-central/rev/b29daa46443b30612415c35be0a3c9c13b9dc5f6/dom/storage/StorageIPC.cpp#569

Yeah, and maybe also disable the test that checks the low disk space stuff.
Flags: needinfo?(jvarga)

This bug is no longer relevant. There have been no test failures for two years.

Status: REOPENED → RESOLVED
Closed: 6 years ago4 years ago
Resolution: --- → INACTIVE
You need to log in before you can comment on or make changes to this bug.