Closed Bug 1421313 Opened 2 years ago Closed 2 years ago

Crash in wcsrtombs

Categories

(Firefox for Android :: General, defect, P1, critical)

Firefox 59
Unspecified
Android
defect

Tracking

()

RESOLVED FIXED
Firefox 59
Tracking Status
firefox57 --- unaffected
firefox58 --- wontfix
firefox59 + fixed

People

(Reporter: marcia, Assigned: snorp)

References

Details

(4 keywords)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is
report bp-12bcdc0c-67e0-4fc7-b453-0c7ea0171128.
=============================================================

Seen while looking at 58 beta crash stats: http://bit.ly/2BjHXMi. Affects 59 and 58. #7 top crash in the most recent b5.

Comments:

*while playing quizzclub trivia I was reading the response to the first question when the crash happened 

Top 10 frames of crashing thread:

0 libxul.so wcsrtombs 
1 libxul.so nsBaseHashtable<nsRefPtrHashKey<nsAtom>, EventNameMapping, EventNameMapping>::Get xpcom/ds/nsTHashtable.h:135
2 libxul.so nsContentUtils::IsEventAttributeName dom/base/nsContentUtils.cpp:4440
3 libxul.so nsGenericHTMLElement::GetWidthHeightForImage dom/html/nsGenericHTMLElement.cpp
4 libxul.so nsGenericHTMLElement::AfterSetAttr dom/html/nsGenericHTMLElement.cpp:707
5 libxul.so nsAttrAndChildArray::SetAndSwapAttr dom/base/nsAttrAndChildArray.cpp:408
6 libxul.so nsAttrValueOrString::IsEmpty dom/base/nsAttrValueOrString.h:93
7 libxul.so mozilla::dom::Element::SetAttrAndNotify dom/base/Element.cpp:2797
8 libmozglue.so arena_t::DallocSmall memory/build/rb.h:150
9 libmozglue.so arena_dalloc memory/build/mozjemalloc.cpp:3605

=============================================================
ni on snorp - any ideas why this would have spiked in 58.0b5?
Flags: needinfo?(snorp)
Crash Signature: [@ wcsrtombs] → [@ wcsrtombs] [ @0x0 | wcsrtombs]
Duplicate of this bug: 1422162
This is the #1 Android crash on the 20171129111022 Android Nightly, with 56% of all crashes, between the two signatures, from a variety of install times.
Keywords: regression, topcrash
The crashes I'm seeing on Nightly look more graphics related than the signature in comment 1:

bp-34f02a2a-f2bb-491a-b28a-7c3590171130

0  @0xfe1701f2 
1 libxul.so wcsrtombs 
2 libxul.so mozilla::gl::TexturePoolOGL::Fill gfx/layers/opengl/TexturePoolOGL.cpp:104
3 libxul.so mozilla::layers::CompositorOGL::BeginFrame gfx/layers/opengl/CompositorOGL.cpp:677
4 libxul.so mozilla::layers::CompositorOGL::ClearRect gfx/layers/opengl/CompositorOGL.cpp:624
5 libxul.so mozilla::layers::LayerManagerComposite::Render gfx/layers/composite/LayerManagerComposite.cpp:917
6 libxul.so mozilla::layers::ContainerLayer::DefaultComputeEffectiveTransforms gfx/layers/Layers.cpp:1324
Crash Signature: [@ wcsrtombs] [ @0x0 | wcsrtombs] → [@ wcsrtombs] [ @0x0 | wcsrtombs]
Crash Signature: [@ wcsrtombs] [ @0x0 | wcsrtombs] → [@ wcsrtombs] [@ @0x0 | wcsrtombs ]
See Also: → 1421960
This signature is pretty high on nightly - we are getting an average of over 100 crashes per day in two of the signatures on nightly.
I'm getting this crash myself now, no idea what would've changed.
Flags: needinfo?(snorp)
Assignee: nobody → snorp
Priority: -- → P1
[Tracking Requested - why for this release]:

Still the #1 Android topcrash by a mile in Nightly 20171201100111.
(In reply to Nicholas Nethercote [:njn] from comment #7)
> [Tracking Requested - why for this release]:
> 
> Still the #1 Android topcrash by a mile in Nightly 20171201100111.

Tracking 59+ for this top crash.
We don't even use the crashing code anymore so let's just remove it.
Attachment #8934176 - Flags: review?(jgilbert) → review?(nical.bugzilla)
Comment on attachment 8934176 [details]
Bug 1421313 - Remove TexturePoolOGL

https://reviewboard.mozilla.org/r/205120/#review211484
Attachment #8934176 - Flags: review?(nical.bugzilla) → review+
Backout by jwillcox@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/06f2b3ee174e
Back out because of build breakage on a CLOSED TREE r=me
https://hg.mozilla.org/mozilla-central/rev/d03fbd96113d
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Perhaps we should consider this for 58 beta once we confirm the fix on nightly.
Yeah, I just installed the latest Nightly and it seems to be fixed. I'll request uplift.
Comment on attachment 8934176 [details]
Bug 1421313 - Remove TexturePoolOGL

Approval Request Comment
[Feature/Bug causing the regression]: Unknown
[User impact if declined]: Crashes, mostly when using Custom Tabs together with Fennec
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: Couldn't hurt. STR that mostly seems to work is open Fennec, then open a custom tab from the app of your choice (such as gmail).
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Only removes unused code (except for the code that was crashing)
[String changes made/needed]: None
Attachment #8934176 - Flags: approval-mozilla-beta?
Looks like the patch doesn't apply to beta:    

grafting 440199:b9123a220811 "Bug 1421313 - Remove TexturePoolOGL r=nical"
local [local] changed gfx/layers/opengl/TexturePoolOGL.cpp which other [graft] deleted
use (c)hanged version, (d)elete, or leave (u)nresolved? merging gfx/layers/opengl/CompositorOGL.cpp
merging gfx/thebes/gfxPlatform.cpp
 warning: conflicts while merging gfx/layers/opengl/CompositorOGL.cpp! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')
Flags: needinfo?(snorp)
Comment on attachment 8934176 [details]
Bug 1421313 - Remove TexturePoolOGL

fennec crash fix, beta58+
Attachment #8934176 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
the conflict resolution looks easy enough...
Flags: needinfo?(snorp)
Backed out for crashes in various Android test suites.
https://hg.mozilla.org/releases/mozilla-beta/rev/8409d8b905182bd075f9a93ae022baf118fe8072

https://treeherder.mozilla.org/logviewer.html#?job_id=150885452&repo=mozilla-beta

PROCESS-CRASH | dom/media/mediasource/test/test_BufferedSeek.html | application crashed [@ mozilla::jni::Accessor::EndAccess<mozilla::java::AndroidGamepadManager::OnGamepadAdded_t> + 0x1e]
Flags: needinfo?(snorp)
Could not reproduce this issue on the latest beta build, 58.0b11.

In our testing session we tried this several times on Sony Xperia Z2 (Android 5.0.2), Xiaomi mi Pad 2 (Android 5.1) x86 build and on a Motorola Nexus 6 (Android 7.1.1).
Duplicate of this bug: 1422794
Ok, let's try landing this one on beta again, but we need 1395497 and 1413500 to come too.
Flags: needinfo?(snorp) → needinfo?(jcristau)
Duplicate of this bug: 1421960
Any luck?
This slightly improved performance on Android 6.0:

== Change summary for alert #11057 (as of Wed, 03 Jan 2018 07:50:07 GMT) ==

Improvements:

  2%  remote-nytimes android-6-0-armv8-api16 opt      1,097.52 -> 1,070.97

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=11057
Comment on attachment 8934176 [details]
Bug 1421313 - Remove TexturePoolOGL

Per comment #65 on bug 1395497, bug 1395497 is required to uplift this one. However, the patch of bug 1395497 is a bit risky at this moment. Let's let it ride the train on 59.
Attachment #8934176 - Flags: approval-mozilla-beta+ → approval-mozilla-beta-
Flags: needinfo?(jcristau)
This is still around in 60.0.1 (#4 overall browser) and affects various devices/APIs. I think all the patches noted in Comment 31 would be in 60.0.1 - are we still expecting to see crashes or should I file another bug?
Flags: needinfo?(snorp)
We probably need a new bug. This signature is pretty much useless, as it's really a symbol in Android that we can't resolve. We should probably skip it so all of the various stacks under this frame get separated out.
Flags: needinfo?(snorp)
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #33)
> We probably need a new bug. This signature is pretty much useless, as it's
> really a symbol in Android that we can't resolve. We should probably skip it
> so all of the various stacks under this frame get separated out.

Filed Bug 1465908 to add it to the skip list.
You need to log in before you can comment on or make changes to this bug.