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

Status: RESOLVED FIXED

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

Description

[Jesse Schwartzentruber (:truber)]

Attachment: 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

Comment 1

[Jesse Schwartzentruber (:truber)]

Attachment: stacktrace

Comment 2

[JW Wang [:jwwang] [:jw_wang]]

Hi Benjamin,
Please check this issue. Thanks!

Comment 3

[Benjamin Chen [:bechen]]

The function TextTrack::RemoveCue doesn't check the "remove target cue" belongs to this TextTrack. I need to fix it and uplift.

Comment 4

[Benjamin Chen [:bechen]]

Attachment: Bug 1304948 - Check the target cue belongs to corresponding TextTrack.

Review commit: https://reviewboard.mozilla.org/r/81060/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/81060/

Comment 5

[Ralph Giles (:rillian)]

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/

Comment 6

[Benjamin Chen [:bechen]]

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 7

[Benjamin Chen [:bechen]]

Attachment: Bug 1304948 - part3: Add testcase.

Review commit: https://reviewboard.mozilla.org/r/82078/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/82078/

Comment 8

[Benjamin Chen [:bechen]]

Attachment: Bug 1304948 - part2: Don't run TimeMarchesOn when shutdown.

Review commit: https://reviewboard.mozilla.org/r/82080/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/82080/

Comment 9

[Benjamin Chen [:bechen]]

Comment on attachment 8794729 [details]
Bug 1304948 - Check the target cue belongs to corresponding TextTrack.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81060/diff/1-2/

Comment 10

[Ralph Giles (:rillian)]

Comment on attachment 8796107 [details]
Bug 1304948 - part3: Add testcase.

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

Thanks!

Comment 11

[Ralph Giles (:rillian)]

Comment on attachment 8796108 [details]
Bug 1304948 - part2: Don't run TimeMarchesOn when shutdown.

https://reviewboard.mozilla.org/r/82080/#review81002

Comment 12

[Pulsebot]

Pushed by [e-mail]:
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

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/720c7b307d0f
https://hg.mozilla.org/mozilla-central/rev/f4907801ba06
https://hg.mozilla.org/mozilla-central/rev/509bdef4e93c

Comment 14

[Benjamin Chen [:bechen]]

Attachment: bug1304948.aurora.patch

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

Comment 15

[Gerry Chang [:gchang]]

Comment on attachment 8797973 [details] [diff] [review]
bug1304948.aurora.patch

Fix a crash. Take it in 51 aurora.

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b6893cd57ff2

Comment 17

[Benjamin Chen [:bechen]]

Attachment: bug1304948.beta.patch

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

Comment 19

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8801966 [details] [diff] [review]
bug1304948.beta.patch

Crash fix, Beta50+

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/69c68bce430d