Closed Bug 1304948 Opened 3 years ago Closed 3 years ago

SEGV near null in [@mozilla::dom::TextTrack::GetTrackElement]

Categories

(Core :: Audio/Video: Playback, defect, P1, critical)

defect

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox50 --- fixed
firefox51 --- fixed
firefox52 --- fixed

People

(Reporter: truber, Assigned: bechen)

References

(Blocks 1 open bug)

Details

(Keywords: crash, csectype-nullptr, testcase)

Attachments

(7 files)

Attached file testcase
The attached testcase crashes in mozilla-central revision 058cf01f6cf2.

==31628==ERROR: AddressSanitizer: SEGV on unknown address 0x0000000000a8 (pc 0x7fc63941046b bp 0x7ffdaa3d17b0 sp 0x7ffdaa3d17b0 T0)
    #0 0x7fc63941046a in get src/obj-firefox/dist/include/mozilla/RefPtr.h:271:27
    #1 0x7fc63941046a in operator mozilla::dom::HTMLTrackElement * src/obj-firefox/dist/include/mozilla/RefPtr.h:287
    #2 0x7fc63941046a in mozilla::dom::TextTrack::GetTrackElement() src/dom/media/TextTrack.cpp:275
    #3 0x7fc6390accc9 in TrackChildPosition src/dom/html/TextTrackManager.cpp:493:38
    #4 0x7fc6390accc9 in mozilla::dom::CompareSimpleTextTrackEvents::LessThan(mozilla::dom::SimpleTextTrackEvent*, mozilla::dom::SimpleTextTrackEvent*) const src/dom/html/TextTrackManager.cpp:519
    #5 0x7fc63908ac5a in operator()<RefPtr<mozilla::dom::SimpleTextTrackEvent> > src/obj-firefox/dist/include/nsTArray.h:813:9
    #6 0x7fc63908ac5a in BinarySearchIf<nsTArray_Impl<RefPtr<mozilla::dom::SimpleTextTrackEvent>, nsTArrayInfallibleAllocator>, detail::ItemComparatorFirstElementGT<mozilla::dom::SimpleTextTrackEvent *&, mozilla::dom::CompareSimpleTextTrackEvents> > src/obj-firefox/dist/include/mozilla/BinarySearch.h:80
    #7 0x7fc63908ac5a in IndexOfFirstElementGt<mozilla::dom::SimpleTextTrackEvent *&, mozilla::dom::CompareSimpleTextTrackEvents> src/obj-firefox/dist/include/nsTArray.h:1462
    #8 0x7fc63908ac5a in RefPtr<mozilla::dom::SimpleTextTrackEvent>* nsTArray_Impl<RefPtr<mozilla::dom::SimpleTextTrackEvent>, nsTArrayInfallibleAllocator>::InsertElementSorted<mozilla::dom::SimpleTextTrackEvent*&, mozilla::dom::CompareSimpleTextTrackEvents, nsTArrayInfallibleAllocator>(mozilla::dom::SimpleTextTrackEvent*&, mozilla::dom::CompareSimpleTextTrackEvents const&) src/obj-firefox/dist/include/nsTArray.h:1481
    #9 0x7fc639089058 in mozilla::dom::TextTrackManager::TimeMarchesOn() src/dom/html/TextTrackManager.cpp:747:7
Attached file stacktrace
Component: DOM → Audio/Video
Hi Benjamin,
Please check this issue. Thanks!
Assignee: nobody → bechen
Component: Audio/Video → Audio/Video: Playback
The function TextTrack::RemoveCue doesn't check the "remove target cue" belongs to this TextTrack. I need to fix it and uplift.
Comment on attachment 8794729 [details]
Bug 1304948 - Check the target cue belongs to corresponding TextTrack.

https://reviewboard.mozilla.org/r/81060/#review79788

Please also add the testcase under dom/media/tests/crashtests/
Attachment #8794729 - Flags: review?(giles) → review+
Comment on attachment 8794729 [details]
Bug 1304948 - Check the target cue belongs to corresponding TextTrack.

https://reviewboard.mozilla.org/r/81060/#review80554

::: dom/media/TextTrack.cpp:1
(Diff revision 1)
>  /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */

When running crashtest, I hit

[Child 8530] ###!!! ASSERTION: Failed NS_DispatchToMainThread() in shutdown; leaking: 'false', file /home/benjamin/hg/mozilla-central/xpcom/glue/nsThreadUtils.cpp, line 185
Could not determine endianness of /home/benjamin/hg/mozilla-central/objdir-linux/dist/bin/libxul.so
#01: mozilla::dom::TextTrackManager::TimeMarchesOn() (/home/benjamin/hg/mozilla-central/dom/html/TextTrackManager.cpp:771 (discriminator 2))
#02: mozilla::dom::HTMLMediaElement::FireTimeUpdate(bool) (/home/benjamin/hg/mozilla-central/dom/html/HTMLMediaElement.cpp:5611)
#03: mozilla::dom::HTMLMediaElement::Pause(mozilla::ErrorResult&) (/home/benjamin/hg/mozilla-central/dom/html/HTMLMediaElement.cpp:2078)
#04: mozilla::binding_danger::TErrorResult<mozilla::binding_danger::AssertAndSuppressCleanupPolicy>::StealNSResult() (/home/benjamin/hg/mozilla-central/objdir-linux/dist/include/mozilla/ErrorResult.h:186)
#05: mozilla::dom::HTMLMediaElement::UnbindFromTree(bool, bool) (/home/benjamin/hg/mozilla-central/dom/html/HTMLMediaElement.cpp:3666)
#06: mozilla::dom::Element::UnbindFromTree(bool, bool) (/home/benjamin/hg/mozilla-central/dom/base/Element.cpp:1935)
#07: nsGenericHTMLElement::UnbindFromTree(bool, bool) (/home/benjamin/hg/mozilla-central/dom/html/nsGenericHTMLElement.cpp:517)
#08: mozilla::dom::Element::UnbindFromTree(bool, bool) (/home/benjamin/hg/mozilla-central/dom/base/Element.cpp:1935)
#09: nsGenericHTMLElement::UnbindFromTree(bool, bool) (/home/benjamin/hg/mozilla-central/dom/html/nsGenericHTMLElement.cpp:517)
#10: mozilla::dom::HTMLSharedElement::UnbindFromTree(bool, bool) (/home/benjamin/hg/mozilla-central/dom/html/HTMLSharedElement.cpp:316)
#11: nsDocument::cycleCollection::Unlink(void*) (/home/benjamin/hg/mozilla-central/dom/base/nsDocument.cpp:1802)
#12: RefPtr<mozilla::dom::HTMLAllCollection>::assign_assuming_AddRef(mozilla::dom::HTMLAllCollection*) (/home/benjamin/hg/mozilla-central/objdir-linux/dist/include/mozilla/RefPtr.h:62)
Comment on attachment 8796107 [details]
Bug 1304948 - part3: Add testcase.

https://reviewboard.mozilla.org/r/82078/#review81000

Thanks!
Attachment #8796107 - Flags: review?(giles) → review+
Comment on attachment 8796108 [details]
Bug 1304948 - part2: Don't run TimeMarchesOn when shutdown.

https://reviewboard.mozilla.org/r/82080/#review81002
Attachment #8796108 - Flags: review?(giles) → review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/720c7b307d0f
Part 1: Check the target cue belongs to corresponding TextTrack. r=rillian
https://hg.mozilla.org/integration/autoland/rev/f4907801ba06
Part 2: Don't run TimeMarchesOn when shutdown. r=rillian
https://hg.mozilla.org/integration/autoland/rev/509bdef4e93c
Part 3: Add testcase. r=rillian
Keywords: checkin-needed
Approval Request Comment
[Feature/regressing bug #]: 882718
[User impact if declined]: crash as testcase.
[Describe test coverage new/current, TreeHerder]: new crash-test
[Risks and why]: low risk, fix is simple
[String/UUID change made/needed]: none
Attachment #8797973 - Flags: approval-mozilla-aurora?
Comment on attachment 8797973 [details] [diff] [review]
bug1304948.aurora.patch

Fix a crash. Take it in 51 aurora.
Attachment #8797973 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Approval Request Comment
[Feature/regressing bug #]: 882718
[User impact if declined]: crash as testcase., bug1310162
[Describe test coverage new/current, TreeHerder]: new crash-test at central
[Risks and why]: low risk, fix is simple
[String/UUID change made/needed]: none
Attachment #8801966 - Flags: approval-mozilla-beta?
Duplicate of this bug: 1310162
Comment on attachment 8801966 [details] [diff] [review]
bug1304948.beta.patch

Crash fix, Beta50+
Attachment #8801966 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
See Also: → 1319486
Severity: normal → critical
You need to log in before you can comment on or make changes to this bug.