Closed Bug 1077402 Opened 8 years ago Closed 8 years ago

[B2G][Flame] Compositor monitor assertion failure (AssertCurrentThreadInMonitor)


(Core :: Panning and Zooming, defect)

Gonk (Firefox OS)
Not set



Tracking Status
firefox33 --- unaffected
firefox34 --- fixed
firefox35 --- fixed
b2g-v2.0 --- unaffected
b2g-v2.1 --- fixed
b2g-v2.2 --- fixed


(Reporter: aosmond, Assigned: aosmond)




(1 file, 1 obsolete file)

Crash in the compositor encountered on b2g-inbound debug builds for flame:

(gdb) bt
#0  tgkill () at bionic/libc/arch-arm/bionic/tgkill.S:46
#1  0xb6f0b13c in pthread_kill (t=<optimized out>, sig=6) at bionic/libc/bionic/pthread_kill.cpp:49
#2  0xb6f0b350 in raise (sig=6) at bionic/libc/bionic/raise.cpp:32
#3  0xb6f0a086 in __libc_android_abort () at bionic/libc/bionic/abort.cpp:55
#4  0xb6f199a0 in abort () at bionic/libc/arch-arm/bionic/abort_arm.S:41
#5  0xb6a6c9f4 in PR_Assert (s=0xb6aeab50 "mon->entryCount != 0 && pthread_equal(mon->owner, pthread_self())", file=<optimized out>, ln=ln@entry=572) at ../../../../../../b2g-inbound/nsprpub/pr/src/io/prlog.c:554
#6  0xb6a7f05a in PR_AssertCurrentThreadInMonitor (mon=0xad64d820) at ../../../../../../b2g-inbound/nsprpub/pr/src/pthreads/ptsynch.c:571
#7  0xb49497a0 in AssertCurrentThreadIn (this=0xafd7d180) at ../../dist/include/mozilla/ReentrantMonitor.h:129
#8  mozilla::layers::AsyncPanZoomController::GetFrameMetrics (this=0xafd7d000) at ../../../../b2g-inbound/gfx/layers/apz/src/AsyncPanZoomController.cpp:2812
#9  0xb4949a88 in mozilla::layers::Axis::GetFrameMetrics (this=this@entry=0xafd7d450) at ../../../../b2g-inbound/gfx/layers/apz/src/Axis.cpp:369
#10 0xb4949acc in mozilla::layers::Axis::GetCompositionLength (this=this@entry=0xafd7d450) at ../../../../b2g-inbound/gfx/layers/apz/src/Axis.cpp:345
#11 0xb4949bde in mozilla::layers::AsyncPanZoomController::GetOverscrollTransform (this=0xafd7d000) at ../../../../b2g-inbound/gfx/layers/apz/src/AsyncPanZoomController.cpp:2474
#12 0xb4966954 in mozilla::layers::ApplyAsyncTransformToScrollbarForContent (aScrollbar=0xaf800d10, aContent=..., aScrollbarIsDescendant=<optimized out>) at ../../../../b2g-inbound/gfx/layers/composite/AsyncCompositionManager.cpp:739
#13 0xb4966b24 in mozilla::layers::AsyncCompositionManager::AlignFixedAndStickyLayers (this=0xafd7d450, aLayer=0xafd7d000, aTransformedSubtreeRoot=0xaf800ca0, aTransformScrollId=12517308902844220416, aPreviousTransformForRoot=..., 
    aCurrentTransformForRoot=..., aFixedLayerMargins=...) at ../../../../b2g-inbound/gfx/layers/composite/AsyncCompositionManager.cpp:261

I reproduced within 1 minute after boot every time and can provide a core file if further analysis is required.
Attached patch bug1077402.patch (obsolete) — Splinter Review
Add monitor locking for accessing protected properties.
Attachment #8499536 - Flags: review?(bugmail.mozilla)
FYI, this seems to have been introduced by bug 1043859.
Depends on: 1043859
Comment on attachment 8499536 [details] [diff] [review]

Review of attachment 8499536 [details] [diff] [review]:

::: gfx/layers/apz/src/AsyncPanZoomController.cpp
@@ +2473,5 @@
> +    ReentrantMonitorAutoEnter lock(mMonitor);
> +    compositionSize.SizeTo(mX.GetCompositionLength(), mY.GetCompositionLength());
> +  }
> +  float scaleX = 1 + kStretchFactor * fabsf(mX.GetOverscroll()) / compositionSize.width;
> +  float scaleY = 1 + kStretchFactor * fabsf(mY.GetOverscroll()) / compositionSize.height;

The only thing I was not certain on was whether GetOverscroll should be protected as well. It does not have an assert in its accessor but that may just be incidental :).
Assignee: nobody → aosmond
Blocks: 1043859
No longer depends on: 1043859
Comment on attachment 8499536 [details] [diff] [review]

Review of attachment 8499536 [details] [diff] [review]:

Thanks for the patch! I think it would be better to just stick lock at the top of the function so everything in it is protected. Functions like Axis::IsOverscrolled and Axis::GetOverscroll should be protected as well. r=me with that.
Attachment #8499536 - Flags: review?(bugmail.mozilla) → review+
Moved lock to the top of function as per review.
Attachment #8499536 - Attachment is obsolete: true
Attachment #8499553 - Flags: review+
Keywords: checkin-needed
Target Milestone: --- → mozilla35
Version: unspecified → Trunk
Wait until the Try run is actually finished before setting the flags please.
Keywords: checkin-needed
My bad, sorry. Done now!
Keywords: checkin-needed
Closed: 8 years ago
Resolution: --- → FIXED
Duplicate of this bug: 1077619
Comment on attachment 8499553 [details] [diff] [review]
bug1077402.patch, v2

Approval Request Comment
[Feature/regressing bug #]: bug 1043859, which is pending aurora approval
[User impact if declined]: debug builds will assert and crash when going into overscroll
[Describe test coverage new/current, TBPL]: verified on m-c by local testing
[Risks and why]: low-risk, only needs uplift if 1043859 is uplifted.
[String/UUID change made/needed]: none
Attachment #8499553 - Flags: approval-mozilla-aurora?
Attachment #8499553 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.