Closed Bug 1145230 Opened 9 years ago Closed 9 years ago

Segfault in mozilla::GStreamerReader::ElementAddedCb

Categories

(Core :: Audio/Video, defect)

x86_64
Linux
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla41
Tracking Status
firefox40 + verified
firefox41 --- verified
firefox-esr38 40+ verified
relnote-firefox --- 40+

People

(Reporter: philippovmi, Assigned: philippovmi)

References

Details

(Keywords: crash, regression, topcrash-linux)

Crash Data

Attachments

(2 files, 2 obsolete files)

Attached patch sugested patch (obsolete) — Splinter Review
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:39.0) Gecko/20100101 Firefox/39.0
Build ID: 20150319185759

Steps to reproduce:

Compiled against revision 234403:cbd0efcd976c. Started browser and opened page https://music.yandex.ru/artist/191206/tracks , clicked a `Yoda' track.


Actual results:

Firefox crashed with SIGSEGV segmentation fault:

(firefox:27725): GStreamer-CRITICAL **: gst_plugin_feature_get_name: assertion `GST_IS_PLUGIN_FEATURE (feature)' failed

Program received signal SIGSEGV, Segmentation fault.

gdb shows the following stack trace:

(gdb) bt
#0  0x0000003599d26cd6 in __strcmp_ssse3 () from /lib64/libc.so.6
#1  0x00007ffff31d4375 in mozilla::GStreamerReader::ElementAddedCb (aPlayBin=<optimized out>, aElement=0x7fffce2b21e0, aUserData=0x7fffb3280000)
    at /home/maxim/projects/mozilla/dom/media/gstreamer/GStreamerReader.cpp:235
#2  0x000000331720b98e in g_closure_invoke () from /lib64/libgobject-2.0.so.0
#3  0x000000331721f947 in ?? () from /lib64/libgobject-2.0.so.0
#4  0x0000003317220de6 in g_signal_emit_valist () from /lib64/libgobject-2.0.so.0
#5  0x00000033172213a3 in g_signal_emit () from /lib64/libgobject-2.0.so.0
#6  0x0000003488831564 in ?? () from /usr/lib64/libgstreamer-0.10.so.0
#7  0x00007fffb01d2438 in ?? () from /usr/lib64/gstreamer-0.10/libgstplaybin.so
#8  0x000000331720b98e in g_closure_invoke () from /lib64/libgobject-2.0.so.0
#9  0x000000331721f947 in ?? () from /lib64/libgobject-2.0.so.0
#10 0x0000003317220de6 in g_signal_emit_valist () from /lib64/libgobject-2.0.so.0
#11 0x00000033172213a3 in g_signal_emit () from /lib64/libgobject-2.0.so.0
#12 0x0000003488844591 in gst_element_add_pad () from /usr/lib64/libgstreamer-0.10.so.0
#13 0x000000331720b98e in g_closure_invoke () from /lib64/libgobject-2.0.so.0
#14 0x000000331721f947 in ?? () from /lib64/libgobject-2.0.so.0
#15 0x0000003317220de6 in g_signal_emit_valist () from /lib64/libgobject-2.0.so.0
#16 0x00000033172213a3 in g_signal_emit () from /lib64/libgobject-2.0.so.0
#17 0x00007fffafb7a148 in ?? () from /usr/lib64/gstreamer-0.10/libgstdecodebin2.so
#18 0x00007fffafb7b430 in ?? () from /usr/lib64/gstreamer-0.10/libgstdecodebin2.so
#19 0x0000003488856f34 in ?? () from /usr/lib64/libgstreamer-0.10.so.0
#20 0x00000034888593f8 in gst_pad_push_event () from /usr/lib64/libgstreamer-0.10.so.0
#21 0x00007fffa64fa1ea in ?? () from /usr/lib64/gstreamer-0.10/libgstmad.so
#22 0x0000003488858e16 in gst_pad_send_event () from /usr/lib64/libgstreamer-0.10.so.0
#23 0x00000034888592c3 in gst_pad_push_event () from /usr/lib64/libgstreamer-0.10.so.0
#24 0x00007fffaf756f6f in ?? () from /usr/lib64/gstreamer-0.10/libgstmpegaudioparse.so
#25 0x00007fffaf758079 in ?? () from /usr/lib64/gstreamer-0.10/libgstmpegaudioparse.so
#26 0x000000348885b71d in ?? () from /usr/lib64/libgstreamer-0.10.so.0
#27 0x000000348885bfde in ?? () from /usr/lib64/libgstreamer-0.10.so.0
#28 0x00007fffaffb03ce in ?? () from /usr/lib64/gstreamer-0.10/libgstcoreelements.so
#29 0x000000348885b71d in ?? () from /usr/lib64/libgstreamer-0.10.so.0
#30 0x000000348885bfde in ?? () from /usr/lib64/libgstreamer-0.10.so.0
#31 0x000000348885b71d in ?? () from /usr/lib64/libgstreamer-0.10.so.0
#32 0x000000348885bfde in ?? () from /usr/lib64/libgstreamer-0.10.so.0
#33 0x000000348a81ded1 in ?? () from /usr/lib64/libgstbase-0.10.so.0
#34 0x000000348888519e in ?? () from /usr/lib64/libgstreamer-0.10.so.0
#35 0x000000359b466d4b in ?? () from /lib64/libglib-2.0.so.0
#36 0x000000359b464e84 in ?? () from /lib64/libglib-2.0.so.0
#37 0x000000359a007761 in start_thread () from /lib64/libpthread.so.0
#38 0x0000003599ce098d in clone () from /lib64/libc.so.6

Looks like gstreamer element factory returned by a `gst_element_get_factory' is NULL and so is a `name' variable, because

(gdb) fr 1
#1  0x00007ffff31d4375 in mozilla::GStreamerReader::ElementAddedCb (aPlayBin=<optimized out>, aElement=0x7fffce2b21e0, aUserData=0x7fffb3280000)
    at /home/maxim/projects/mozilla/dom/media/gstreamer/GStreamerReader.cpp:235
235       if (!strcmp(name, "uridecodebin")) {
(gdb) l
230                                          gpointer aUserData)
231     {
232       const gchar *name =
233         gst_plugin_feature_get_name(GST_PLUGIN_FEATURE(gst_element_get_factory(aElement)));
234
235       if (!strcmp(name, "uridecodebin")) {
236         g_signal_connect(G_OBJECT(aElement), "autoplug-sort",
237                          G_CALLBACK(GStreamerReader::ElementFilterCb), aUserData);
238       }
239     }
(gdb) p name
$1 = <optimized out>
(gdb) p *aElement
$2 = {object = {object = {g_type_instance = {g_class = 0x7fffb7cbda00}, ref_count = 2, qdata = 0x0}, refcount = 0, lock = 0x7fffcf2da820, 
    name = 0x7fffb340d9a0 "playbin2inputselector0", name_prefix = 0x0, parent = 0x7fffe254b000, flags = 0, _gst_reserved = 0x0}, state_lock = 0x7fffb7cd9e80, 
  state_cond = 0x7fffcf2ffa00, state_cookie = 0, current_state = GST_STATE_NULL, next_state = GST_STATE_VOID_PENDING, pending_state = GST_STATE_VOID_PENDING, 
  last_return = GST_STATE_CHANGE_SUCCESS, bus = 0x7fffcc963580, clock = 0x0, base_time = 0, numpads = 1, pads = 0x7fffb340d920, numsrcpads = 1, srcpads = 0x7fffb340d900, 
  numsinkpads = 0, sinkpads = 0x0, pads_cookie = 1, abidata = {ABI = {target_state = GST_STATE_NULL, start_time = 0}, _gst_reserved = {0x1, 0x0, 0x0, 0x0}}}
(gdb) p gst_element_get_factory(aElement)
$3 = (GstElementFactory *) 0x0

Attached `nil_gst_factory.patch' patch fixes an issue for my case.


Expected results:

yo-yo-yo-yo-yoda =)
This is a media playback bug
Component: WebRTC: Audio/Video → Video/Audio
Attachment #8580142 - Flags: review?(cajbir.bugzilla)
Attachment #8580142 - Flags: review?(cajbir.bugzilla) → review+
Thank You for reviewing a patch.

Should I change a patch description to include an issue id? I generated patch before submitting an issue, so I didn't know its id.
Assignee: nobody → philippovmi
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
(In reply to Maxim Philippov from comment #2)
> Thank You for reviewing a patch.
> 
> Should I change a patch description to include an issue id? I generated
> patch before submitting an issue, so I didn't know its id.
Flags: needinfo?(cajbir.bugzilla)
Yes, that would be helpful. Something like:

Bug 1145230 - Segfault in mozilla::GStreamerReader::ElementAddedCB - r=cajbir
Flags: needinfo?(cajbir.bugzilla)
Attachment #8580142 - Attachment is obsolete: true
Is there maybe a try-build addressing this issue that I can test? 
I think I'm affected by this bug, as I've reported here it started for me with Nightly and it is now happening with 37.0.1 as well https://bugzilla.mozilla.org/show_bug.cgi?id=1143394
BTW, shouldn't the severity of this bug be risen? In some cases the current *release* version of Firefox is instantly crashing on sites such as youtube and vimeo, making it unfit for regular use.
Severity: normal → critical
can we get a try run for this change ? Thanks!
Flags: needinfo?(philippovmi)
(In reply to Carsten Book [:Tomcat] from comment #8)
> can we get a try run for this change ? Thanks!

Sure. What should I do?
Flags: needinfo?(philippovmi)
Crash-Report on Linux (32Bit, i686) with URL  https://www.youtube.com/watch?v=WCWPmk6n31c  :
https://crash-stats.mozilla.com/report/index/3fe86b31-c610-43a2-aa9a-5191a2150411

The crashes started with Version 37 on a lot of Youtube Videos. I have no Flash-PlugIn and use the HTML5 Mode of Youtube now for years without any problem. In the past i have observed that fresh videos often use the MP4 Format and after 2 or 3 days the same video is delivered in WEBM-Format with VP9-Codec. The WEBM-Video-Format does not trigger this kind of crash but MP4-Videos crash Firefox Version 37 reproducible.
(In reply to Maxim Philippov from comment #9)
> (In reply to Carsten Book [:Tomcat] from comment #8)
> > can we get a try run for this change ? Thanks!
> 
> Sure. What should I do?

Post a link to a trybuild containing a patch, if it is available?
(In reply to msth67 from comment #11)
> (In reply to Maxim Philippov from comment #9)
> > (In reply to Carsten Book [:Tomcat] from comment #8)
> > > can we get a try run for this change ? Thanks!
> > 
> > Sure. What should I do?
> 
> Post a link to a trybuild containing a patch, if it is available?

I don't have required repository access level to use try server, sorry.
I've just observed that whilst the current Nightly (buildID 20150423030204) and Firefox beta 38.0.6 (buildID 20150420134330) are still crashing just like before, interestingly Firefox 37.0.2 (release version) is now no longer crashing for me on Youtube, but it still does on Vimeo.

I've tested with the same YouTube videos and all I can say is that Nightly and beta do crash, but Firefox 37.0.2 does not.
https://bugzilla.mozilla.org/show_bug.cgi?id=981869#c47 has some context which seems to apply here. I guess it is actually the same issue.
(In reply to Georg Koppen from comment #14)
> https://bugzilla.mozilla.org/show_bug.cgi?id=981869#c47 has some context
> which seems to apply here. I guess it is actually the same issue.

Indeed, that's the same issue. FF 37.x & 38.x still crash on many sites with embedded video on systems with gstreamer 10.x. I get crashes multiple times a day on my company PC with CentOS6, where I cannot upgrade gstreamer.

A patch is already available for almost 2 months now; why doesn't it get released?
Attachment #8587269 - Flags: review?(edwin)
Comment on attachment 8587269 [details] [diff] [review]
sugested patch with fixed description

Review of attachment 8587269 [details] [diff] [review]:
-----------------------------------------------------------------

We generally prefer early return style -- |if (!factory) return;| -- otherwise, good.
Attachment #8587269 - Flags: review?(edwin) → review+
Is there a direct link to a tarball here somewhere https://treeherder.mozilla.org/#/jobs?repo=try&revision=866bc89552b6 ? I'm sorry but I can't find it.
I actually meant a compiled, ready-to-run Firefox build with this patch incorporated.
I'd really like to give this a try (provided that it is a 64 bit build) because to date I still have to keep a 36.0.4 version also installed to be able to watch videos.
(In reply to msth67 from comment #19)
> I actually meant a compiled, ready-to-run Firefox build with this patch
> incorporated.
> I'd really like to give this a try (provided that it is a 64 bit build)
> because to date I still have to keep a 36.0.4 version also installed to be
> able to watch videos.

My nightly builds available here http://www.wg9s.com/mozilla/firefox/ include the patch for this bug.
I re-built 38.0.1 ESR on CentOS 6.6 with the patch - you are welcome to try it out if you want <ftp://ftp6.moving-picture.com/private/firefox/firefox-38.0.1.en-US.linux-x86_64.tar.bz2>
Thanks, however if that also requires a libc6 version higher than 2.11 (like the build linked in https://bugzilla.mozilla.org/show_bug.cgi?id=1145230#c20),that's 
unfortunately a no go, because all I have on this system is version 2.11.
CentOS 6 uses glibc 2.12 - so you might be lucky - it might run OK

Otherwise, I guess you may have to rebuild Firefox with this patch yourself ?
Maxim, could you update your patch with the review comments in comment 17 addressed to get it into the correct shape for check-in?
Flags: needinfo?(philippovmi)
Attachment #8587269 - Attachment is obsolete: true
Flags: needinfo?(philippovmi)
> Maxim, could you update your patch with the review comments in comment 17 addressed to get it into the correct shape for check-in?

Certainly, done.
Keywords: checkin-needed
Is this fix going to be applied to 38 ESR ?
This is a duplicate of bug 1143394.
(In reply to mathew.hodson from comment #29)
> This is a duplicate of bug 1143394.

No, this is a subset of bug 1143394.  This fixes just one (of several) gstreamer crashes.
(In reply to Bill Gianopoulos [:WG9s] from comment #30)
> (In reply to mathew.hodson from comment #29)
> > This is a duplicate of bug 1143394.
> 
> No, this is a subset of bug 1143394.  This fixes just one (of several)
> gstreamer crashes.

Although I can no longer duplicate the other crash.  It appears my update from fedora21 to fedora22 provided a newer gstreamer that fixed the other issue.  However, I will point out that although we normally duplicate bugs the way you suggest.  This one has a higher number so we mark it as a duplicate of the lower numbered bug.  In this case, since this bug contains the fix we dupe them the other way around.
https://hg.mozilla.org/mozilla-central/rev/803af11f2eaf
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Is this fix going to go into ESR 38?

I can't really wait until ESR 45 to have this fixed ...
In Nightly 41.0a1 build ID 20150603030208 I no longer see the crash, either on Youtube or Vimeo.
(In reply to James Pearson from comment #33)
> Is this fix going to go into ESR 38?
> 
> I can't really wait until ESR 45 to have this fixed ...

Should I set the tracking-firefox-esr38 flag in this bug?
[Tracking Requested - why for this release]:

Bug causes problems with ESR 38 - can't wait for a fix to appear in ESR 45
Setting status flags for ESR 38 to reflect above comments ^ Tracking because crashes are bad, and a fix would be good before ESR 45.
(In reply to James Pearson from comment #35)
> Should I set the tracking-firefox-esr38 flag in this bug?

Yes. Thanks for doing this. This flags the bug for release management.

(In reply to James Pearson from comment #36)
> [Tracking Requested - why for this release]:
> 
> Bug causes problems with ESR 38 - can't wait for a fix to appear in ESR 45

Makes sense. 

Maxim - What do you think about uplifting this fix to ESR38?
Flags: needinfo?(philippovmi)
Here is a patch for mozilla-esr38 branch. It is quite trivial so it won't cause additional troubles.
Flags: needinfo?(philippovmi)
Attachment #8638923 - Flags: approval-mozilla-esr38?
Comment on attachment 8638923 [details] [diff] [review]
patch for mozilla-esr38 branch

We'll take this in ESR 38.2.0. ESR38+
Attachment #8638923 - Flags: approval-mozilla-esr38? → approval-mozilla-esr38+
It turns out that this null deref is happening in high volume on release 40.

We don't have symbols so it's not obvious from the signature, but nearly all of those libc crashes are due to this issue.

[Tracking Requested - why for this release]: High volume crash on Linux, often near startup. Fixed in v41. I don't know if this is worth its own release, but we should definitely include this patch if we happen to do another.
I estimate that this is about 40% of Linux crashes on version 40.0.
Crash Signature: [@ libc-2.11.1.so@0x11824b ] [@ libc-2.17.so@0x7cd40 ] [@ libc-2.13.so@0x78720 ] [@ libc-2.11.3.so@0x72eb0 ] [@ libc-2.11.3.so@0x72ad0 ] [@ libc-2.13.so@0x724e0 ] [@ libc-2.11.1.so@0x1205cb ] [@ libc-2.13.so@0x75e70 ] [@ libc-2.11.3.so@0x72b98 ]
Edwin, would it be possible to get a fix (patch) ready for this bug that applies on mozilla-release? This bug is a top crash on FF40 on linux. Sylvestre will make a final call on whether we accept the patch for uplift to m-r or not. I was just wondering if it is easy to get the patch ready in the meantime.
Flags: needinfo?(edwin)
Interestingly, it looks like this issue was about a quarter of Linux crashes during the 39 cycle, so we've been shipping this way for a while.
Comment on attachment 8638923 [details] [diff] [review]
patch for mozilla-esr38 branch

Approval Request Comment
[Feature/regressing bug #]:
I don't know why or whether this has increased in frequency recently.
[User impact if declined]:
High frequency null deref crash.
[Describe test coverage new/current, TreeHerder]:
I don't know why this isn't caught by our test coverage.
It may be triggered by differences in different GStreamer versions.
[Risks and why]: 
Very low.  Patch adds two null-checks.
[String/UUID change made/needed]:
None.
Attachment #8638923 - Flags: approval-mozilla-release?
The only differences in the affected file between release and when this was
fixed on central are logging and changes to GetBuffered().  Neither of those
interfere with the patch.  The esr38 patch differs only in offset, so I picked that 
one for 40 because it applies without offset.
Thanks Karl. Removing ni? for Edwin.
Flags: needinfo?(edwin)
Comment on attachment 8638923 [details] [diff] [review]
patch for mozilla-esr38 branch

OK, let's take it in case we do a dot release.
Attachment #8638923 - Flags: approval-mozilla-release? → approval-mozilla-release+
Tracking as it is a top crash.
Keywords: crash
Bug 1143394 is also a duplicate of this bug (same "mozilla::GStreamerReader::ElementAddedCb(...)" in stack trace). Also maybe add its crash signature ([@ libc-2.11.1.so@0x12b346 ]) to this bug's crash signature list.
Crash Signature: [@ libc-2.11.1.so@0x11824b ] [@ libc-2.17.so@0x7cd40 ] [@ libc-2.13.so@0x78720 ] [@ libc-2.11.3.so@0x72eb0 ] [@ libc-2.11.3.so@0x72ad0 ] [@ libc-2.13.so@0x724e0 ] [@ libc-2.11.1.so@0x1205cb ] [@ libc-2.13.so@0x75e70 ] [@ libc-2.11.3.so@0x72b98 ] → [@ libc-2.11.1.so@0x11824b ] [@ libc-2.17.so@0x7cd40 ] [@ libc-2.13.so@0x78720 ] [@ libc-2.11.3.so@0x72eb0 ] [@ libc-2.11.3.so@0x72ad0 ] [@ libc-2.13.so@0x724e0 ] [@ libc-2.11.1.so@0x1205cb ] [@ libc-2.13.so@0x75e70 ] [@ libc-2.11.3.so@0x72b98 ] …
Added to the 40.0.3 release notes with "Fix a segmentation fault in the GStreamer support (GNU/Linux) (1145230)" as wording.
Setting qe-verify+ flag to try and verify this.
Flags: qe-verify+
I can`t reproduce this issue using Ubuntu 14.04 64-bit with 37.0.1 and 40.0.2 where the fix did not end up, so I can`t say this is verified. I loaded a bunch of videos on different websites but with no luck on reproducing.

Requesting needinfo from people that did reproduce in the first place.
Flags: needinfo?(msth67)
(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #62)
> Forgot to mention to use latest RC build 40.0.3 and latest ESR 38.2.1:
> http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/40.0.3-candidates/
> build1/linux-x86_64/en-US/firefox-40.0.3.tar.bz2
> http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/38.2.1esr-candidates/
> build2/linux-x86_64/en-US/firefox-38.2.1esr.tar.bz2
> 
> Also adding another needinfo.

On CentOS 6.6, firefox 40.0.2 crashes when I try to load a quicktime movie (e.g. http://qtdevseed.apple.com/qadrift/_media/_source_files/jpeg_movs/clipcanvas_14348_PhotoJPEG.mov)

I get the error on the terminal:

(firefox:12344): GStreamer-CRITICAL **: gst_plugin_feature_get_name: assertion `GST_IS_PLUGIN_FEATURE (feature)' failed

The above RC 40.0.3 doesn't crash

In both cases, this is just a vanilla install - no plugins etc

This fix is already in firefox ESR 38.2.0 - so that movie doesn't crash ESR 38.2.0 (or 38.2.1 RC)
Flags: needinfo?(james-p)
(In reply to James Pearson from comment #63)
> (In reply to Bogdan Maris, QA [:bogdan_maris] from comment #62)
> > Forgot to mention to use latest RC build 40.0.3 and latest ESR 38.2.1:
> > http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/40.0.3-candidates/
> > build1/linux-x86_64/en-US/firefox-40.0.3.tar.bz2
> > http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/38.2.1esr-candidates/
> > build2/linux-x86_64/en-US/firefox-38.2.1esr.tar.bz2
> > 
> > Also adding another needinfo.
> 
> On CentOS 6.6, firefox 40.0.2 crashes when I try to load a quicktime movie
> (e.g.
> http://qtdevseed.apple.com/qadrift/_media/_source_files/jpeg_movs/
> clipcanvas_14348_PhotoJPEG.mov)
> 
> I get the error on the terminal:
> 
> (firefox:12344): GStreamer-CRITICAL **: gst_plugin_feature_get_name:
> assertion `GST_IS_PLUGIN_FEATURE (feature)' failed
> 
> The above RC 40.0.3 doesn't crash
> 
> In both cases, this is just a vanilla install - no plugins etc
> 
> This fix is already in firefox ESR 38.2.0 - so that movie doesn't crash ESR
> 38.2.0 (or 38.2.1 RC)

Thanks for the verification. Marking this bug as verified fixed.

I left out 41 build though, sorry for that. Would you mind also try latest beta?
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/41.0b5-candidates/build1/linux-x86_64/en-US/firefox-41.0b5.tar.bz2
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Flags: needinfo?(msth67)
Flags: needinfo?(james-p)
(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #64)
> Thanks for the verification. Marking this bug as verified fixed.
> 
> I left out 41 build though, sorry for that. Would you mind also try latest
> beta?
> http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/41.0b5-candidates/
> build1/linux-x86_64/en-US/firefox-41.0b5.tar.bz2

41.0b5 doesn't crash
Flags: needinfo?(james-p)
No crashes for me with 41.0b5 on Linux 64, however I can still see this message 

GLib-GObject-WARNING **: /build/buildd/glib2.0-2.24.1/gobject/gsignal.c:2273: signal `autoplug-sort' is invalid for instance 0x*******

in a terminal.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: