Crash on multipart/x-mixed-replace: gif,jpeg,jpeg sequence [@ nsJPEGDecoder::OutputScanlines][@ gdk_rgb_convert_0888][@ gdk_rgb_init]

RESOLVED FIXED in mozilla1.9alpha8



13 years ago
7 years ago


(Reporter: Paul Marks, Assigned: Andrew Smith)


({crash, testcase})

crash, testcase
Bug Flags:
blocking1.9 +
blocking1.8.1.1 -
blocking1.8.0.9 -

Firefox Tracking Flags

(Not tracked)


(crash signature, URL)


(2 attachments, 4 obsolete attachments)



13 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b2) Gecko/20050622 Firefox/1.0+
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b2) Gecko/20050622 Firefox/1.0+

I created this testcase while trying to write a server-push webcam.  It seems
that when I send a gif, followed by a series of jpegs, it causes an instant
crash for Firefox and Mozilla on Linux.

Reproducible: Always

Steps to Reproduce:
1. Use Linux
2. Goto URL

Actual Results:  
Firefox went poof.

Expected Results:  
It should display some images.

The sequence is generated by a Perl script.  Here's the source:
Assignee: nobody → pavlov
Component: General → ImageLib
Product: Firefox → Core
QA Contact: general
Version: unspecified → Trunk

Comment 1

13 years ago
Created attachment 187286 [details]

Comment 2

13 years ago
confirmed with linux suite trunk 2005062305

==> gfx:gtk
Assignee: pavlov → blizzard
Component: ImageLib → GFX: Gtk
Ever confirmed: true
Keywords: crash, testcase
QA Contact: gtk

Comment 3

13 years ago
This is almost certainly not linux-only.
Over to a real owner...
Assignee: blizzard → pavlov
Flags: blocking1.9a1?


12 years ago
Flags: blocking1.9a1? → blocking1.9+

Comment 5

12 years ago
*** Bug 254912 has been marked as a duplicate of this bug. ***

Comment 6

12 years ago
swift% uname -a
SunOS swift 5.11 snv_49 i86pc i386 i86pc
swift% mdb /usr/lib/firefox/firefox-bin /var/crash/cores/
> $c!c++filt`_lwp_kill+0x15(1, b)`raise+0x22(b)
void nsProfileLock::FatalSignalHandler(int)+0xe6(b, 0, 8045184)`__sighndlr+0xf(b, 0, 8045184, 806aec8)`call_user_handler+0x2b8(b, 0, 8045184)`sigacthandler+0xc2(b, 0, 8045184)`gdk_rgb_convert_0888+0x86(825b100, 8389018, 40, 0, 40, 20)`gdk_draw_rgb_image_core+0x160(825b100, 81b7938, 825b8f0, 0, 0, 40)`gdk_draw_rgb_image_dithalign+0x8e(81b7938, 825b8f0, 0, 0, 40, 20)`void nsImageGTK::UpdateCachedImage()+0x613(f662998)`unsigned nsImageGTK::Draw(nsIRenderingContext&,nsIDrawingSurface*,int,int,int,int,int,int,int,int)+0x6a(f662998, b585e70, db8ea70, 0, 0, 40)`unsigned nsRenderingContextImpl::DrawImage(imgIContainer*,const nsRect&,const nsRect&)+0x528(b585e70, fe73758, 8045840, 8045830)`unsigned nsRenderingContextGTK::DrawImage(imgIContainer*,const nsRect&,const nsRect&)+0x17a(b585e70, fe73758, 8045840, 8045830)`unsigned nsImageFrame::Paint(nsPresContext*,nsIRenderingContext&,const nsRect&,nsFramePaintLayer,unsigned)+0x31b(10522f80, 1030e3d0, b585e70, 8045950, 4, 0)`void nsContainerFrame::PaintChild(nsPresContext*,nsIRenderingContext&,const nsRect&,nsIFrame*,nsFramePaintLayer,unsigned)+0x105(10522910, 1030e3d0, b585e70, 8045bc0, 10522f80, 4)`void nsBlockFrame::PaintChild(nsPresContext*,nsIRenderingContext&,const nsRect&,nsIFrame*,nsFramePaintLayer,unsigned)+0x3e(10522910, 1030e3d0, b585e70, 8045bc0, 10522f80, 4)`void nsBlockFrame::PaintChildren(nsPresContext*,nsIRenderingContext&,const nsRect&,nsFramePaintLayer,unsigned)+0x500(10522910, 1030e3d0, b585e70, 8045bc0, 4, 0)`void nsHTMLContainerFrame::PaintDecorationsAndChildren(nsPresContext*,nsIRenderingContext&,const nsRect&,nsFramePaintLayer,int,unsigned)+0x1a5(10522910, 1030e3d0, b585e70, 8045bc0, 4, 1)`unsigned nsBlockFrame::Paint(nsPresContext*,nsIRenderingContext&,const nsRect&,nsFramePaintLayer,unsigned)+0x156(10522910, 1030e3d0, b585e70, 8045bc0, 4, 0)`void nsContainerFrame::PaintChild(nsPresContext*,nsIRenderingContext&,const nsRect&,nsIFrame*,nsFramePaintLayer,unsigned)+0x105(10522790, 1030e3d0, b585e70, 8045e30, 10522910, 4)`void nsBlockFrame::PaintChild(nsPresContext*,nsIRenderingContext&,const nsRect&,nsIFrame*,nsFramePaintLayer,unsigned)+0x3e(10522790, 1030e3d0, b585e70, 8045e30, 10522910, 4)`void nsBlockFrame::PaintChildren(nsPresContext*,nsIRenderingContext&,const nsRect&,nsFramePaintLayer,unsigned)+0x500(10522790, 1030e3d0, b585e70, 8045e30, 4, 0)`void nsHTMLContainerFrame::PaintDecorationsAndChildren(nsPresContext*,nsIRenderingContext&,const nsRect&,nsFramePaintLayer,int,unsigned)+0x1a5(10522790, 1030e3d0, b585e70, 8045e30, 4, 1)`unsigned nsBlockFrame::Paint(nsPresContext*,nsIRenderingContext&,const nsRect&,nsFramePaintLayer,unsigned)+0x156(10522790, 1030e3d0, b585e70, 8045e30, 4, 0)`void nsContainerFrame::PaintChild(nsPresContext*,nsIRenderingContext&,const nsRect&,nsIFrame*,nsFramePaintLayer,unsigned)+0x105(106b6f64, 1030e3d0, b585e70, 8046090, 10522790, 4)`void nsContainerFrame::PaintChildren(nsPresContext*,nsIRenderingContext&,const nsRect&,nsFramePaintLayer,unsigned)+0x61(106b6f64, 1030e3d0, b585e70, 8046090, 4, 0)`unsigned nsHTMLContainerFrame::Paint(nsPresContext*,nsIRenderingContext&,const nsRect&,nsFramePaintLayer,unsigned)+0x78(106b6f64, 1030e3d0, b585e70, 8046090, 4, 0)`unsigned CanvasFrame::Paint(nsPresContext*,nsIRenderingContext&,const nsRect&,nsFramePaintLayer,unsigned)+0x98(106b6f64, 1030e3d0, b585e70, 8046090, 4, 0)`unsigned PresShell::Paint(nsIView*,nsIRenderingContext&,const nsRect&)+0x1d2(e356ff0, aff1268, b585e70, 8046090)`unsigned nsView::Paint(nsIRenderingContext&,const nsRect&,unsigned,int&)+0x63(aff1268, b585e70, 8046090, 0, 80460ac)`void nsViewManager::RenderDisplayListElement(DisplayListElement2*,nsIRenderingContext*)+0xbe(b8367c8, c756fa0, b585e70)`void nsViewManager::RenderViews(nsView*,nsIRenderingContext&,const nsRegion&,nsIDrawingSurface*,const nsVoidArray&)+0x1ef(b8367c8, db7f740, b585e70, 8046220, db8ea70, 8046270)`void nsViewManager::Refresh(nsView*,nsIRenderingContext*,nsIRegion*,unsigned)+0x556(b8367c8, db7f740, b585e70, f0d38d8, 1)`unsigned nsViewManager::DispatchEvent(nsGUIEvent*,nsEventStatus*)+0x427(b8367c8, 80464b4, 804645c)`nsEventStatus HandleEvent(nsGUIEvent*)+0x8b(80464b4)`unsigned nsCommonWidget::DispatchEvent(nsGUIEvent*,nsEventStatus&)+0x3a(f1ab220, 80464b4, 8046520)`int nsWindow::OnExposeEvent(_GtkWidget*,_GdkEventExpose*)+0x21c(f1ab220, 825b4e8, 8046a0c)`int expose_event_cb(_GtkWidget*,_GdkEventExpose*)+0x4a(825b4e8, 8046a0c, 0)`_gtk_marshal_BOOLEAN__BOXED+0x71(8425fc8, 8046620, 2, 80466dc, 804663c, 0)`g_closure_invoke+0x112(8425fc8, 8046620, 2, 80466dc, 804663c)`signal_emit_unlocked_R+0x754(8154658, 0, 825b4e8, 804685c, 80466dc)`g_signal_emit_valist+0x663(825b4e8, 21, 0, 8046950)`g_signal_emit+0x29(825b4e8, 21, 0, 8046a0c, 8046974)`gtk_widget_event_internal+0x212(825b4e8, 8046a0c)`gtk_widget_send_expose+0x82(825b4e8, 8046a0c)`gtk_main_do_event+0x42c(8046a0c, 0)`gdk_window_process_updates_internal+0x15b(a2f5220)`gdk_window_process_all_updates+0x66(fe563dbc, 8046ab0, fe4fc713, 0, fe563dbc, 8046b38)`gdk_window_update_idle+0x26(0)`g_idle_dispatch+0x1f(e6535e0, fe59e66c, 0)`g_main_dispatch+0x1c8(80c3d10)`g_main_context_dispatch+0x85(80c3d10)`g_main_context_iterate+0x3d1(80c3d10, 1, 1, 81a1b58)`g_main_loop_run+0x1ba(85a0260)`gtk_main+0xb3(8046ee8, 8086670, 8169600, 8046c48, fb758bb3, 816faa8)`unsigned nsAppShell::Run()+0x34(816faa8)`unsigned nsAppStartup::Run()+0x2b(8169600)
XRE_main+0x28b9(5, 8046f94, 8086648)
main+0x27(5, 8046f94, 8046fac)
_start+0x7a(5, 804711c, 8047139, 8047143, 8047145, 8047154)

The reporter of this bug thinks it's a good idea to advertise on slashdot that we haven't fixed this bug. I don't appreciate that.
OS: Linux → All
Flags: blocking1.8.1.1?
Flags: blocking1.8.0.9?
Realistically I don't see a fix appearing in time for the next releases, but we'd like to take a patch when we've got one.
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x+
Flags: blocking1.8.1.1?
Flags: blocking1.8.1.1-
Flags: blocking1.8.0.9?
Flags: blocking1.8.0.9-
We're not ever going to block for this crash on the 1.8.0 branch, but if this ever gets fixed you can ask for approval on a branch-merged patch.
Flags: wanted1.8.1.x?
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x+
Flags: wanted1.8.1.x?
Target Milestone: --- → mozilla1.9alpha6
We'll try to get this in beta 1; however, there's a chance this one might not make it until B2.  Per Stuart.
Target Milestone: mozilla1.9alpha6 → mozilla1.9beta1

Comment 10

11 years ago
In case it's not already known - opening doesn't crash. (that's the data alone, without the html code)

Does anyone know or suspect what's causing this?

Comment 11

11 years ago
And I was going to add that after going to going to also doesn't crash.

Comment 12

11 years ago

Incident ID: 34871520
Stack Signature	nsJPEGDecoder::OutputScanlines() aabf39d4
Product ID	CaminoTrunk
Build ID	2007061722
Trigger Time	2007-08-12 03:13:00.0
Platform	MacOSX
Operating System	Darwin 7.9.0
Module	Camino + (001d9fa8)
URL visited	
User Comments	testing a known fatal (gtk) bug on osx
Since Last Crash	1886 sec
Total Uptime	3329793 sec
Trigger Reason	SIGBUS: Bus Error: (signal 10)
Source File, Line No.	/builds/tinderbox/CmTrunk/Darwin_8.6.0_Depend/build/ppc/modules/libpr0n/decoders/jpeg//builds/tinderbox/CmTrunk/Darwin_8.6.0_Depend/mozilla/modules/libpr0n/decoders/jpeg/nsJPEGDecoder.cpp, line 487
Stack Trace 	
nsJPEGDecoder::OutputScanlines()  [mozilla/modules/libpr0n/decoders/jpeg/nsJPEGDecoder.cpp, line 487]
nsJPEGDecoder::OutputScanlines()  [mozilla/modules/libpr0n/decoders/jpeg/nsJPEGDecoder.cpp, line 477]
imgRequest::OnDataAvailable()  [mozilla/modules/libpr0n/src/imgRequest.cpp, line 912]
nsMultiMixedConv::SendData()  [mozilla/netwerk/streamconv/converters/nsMultiMixedConv.cpp, line 840]
nsMultiMixedConv::OnDataAvailable()  [mozilla/netwerk/streamconv/converters/nsMultiMixedConv.cpp, line 524]
nsStreamListenerTee::OnDataAvailable()  [mozilla/netwerk/base/src/nsStreamListenerTee.cpp, line 840]

This crash is probably easier to deal w/, OTOH fixing one might not fix others.

The obvious guess is that we don't switch decoders when the content type changes.
Assignee: pavlov → nobody
Component: GFX: Gtk → ImageLib
QA Contact: gtk → imagelib
Hardware: PC → All
Summary: Crash on multipart/x-mixed-replace: gif,jpeg,jpeg sequence → Crash on multipart/x-mixed-replace: gif,jpeg,jpeg sequence [@ nsJPEGDecoder::OutputScanlines][@ gdk_rgb_convert_0888][@ gdk_rgb_init]
Please don't nuke assignees, ok?
Assignee: nobody → pavlov

Comment 14

11 years ago
that was bugzilla being "helpful" [automatic js], in fact, there's no easy way to get what i want which is to fix the QA contact as i move. (newer versions of bugzilla allow for this.) I should notice such nukes when i review bugmail....

Comment 15

11 years ago
Created attachment 276678 [details] [diff] [review]
potential solution

The crash happens because in imgContainer::DecodingComplete() the gfxIImageFrame is optimised, and then nsJPEGDecoder::OutputScanlines() was trying to access its data.

There are many ways to fix it but the solution with likely the least impact on the rest of the tree is to not reuse the container. Stuart, what do you think?
Attachment #276678 - Flags: review?(pavlov)

Comment 16

11 years ago
Another 'solution' would be to do SetMutable(TRUE) on re-used images, however that is not implemented.
SetMutable(FALSE) -> image->Optimize(): remove the image surface, replacing it by the OS platform image.
SetMutable(TRUE) -> NOP
To fix this, the SetMutable(TRUE) should call 'LockImagePixels' to recreate the image buffer again.
However this is costly (allocation of image buffer, and then conversion of OS surface data to the image buffer), possible just as costly (or more) than just killing the existing image and creating a new one?

Comment 17

11 years ago
I don't know more about multipart/x-mixed-replace than what I learned from reading for a few minutes on some webpage.

From what I did learn it seems that the old frames are permanently discarded, so doing any work on the image that's just about to be discarded seems to be a waste.

Comment 18

11 years ago
imho, the jpeg decoder should just call LockImageData() on the frame before it accesses the data and calls Unlock after.

Comment 19

11 years ago
Comment on attachment 276678 [details] [diff] [review]
potential solution

this patch will make it so that we don't draw over the same frame which makes animations via multipart mixed really suck
Attachment #276678 - Flags: review?(pavlov) → review-

Comment 20

11 years ago
Created attachment 276863 [details] [diff] [review]
using fixed SetMutable(false)

If I just call mFrame->LockImageData() in OutputScanlines() I get an assertion fired: trying to get data on an immutable frame: 'mMutable', file gfxImageFrame.cpp, line 276

This patch makes SetMutable(false) do something useful (LockImageData()), and I call SetMutable() if it is a multipart. These 6 lines in OutputScanlines() are almost the same as the ones at the bottom of the file.

I'm not entirely sure it's ok to leave the frame in whatever state LockImageData() puts it in, but it seems to work.
Attachment #276678 - Attachment is obsolete: true
Attachment #276863 - Flags: review?(pavlov)

Comment 21

11 years ago
Created attachment 276865 [details] [diff] [review]
 using fixed SetMutable(false)

grr i forgot to rerun diff
Attachment #276863 - Attachment is obsolete: true
Attachment #276865 - Flags: review?(pavlov)
Attachment #276863 - Flags: review?(pavlov)

Comment 22

11 years ago
Comment on attachment 276865 [details] [diff] [review]
 using fixed SetMutable(false)

Unlock releases the image surface if there is an optimized surface so not calling that could result in 2x the amount of memory being used for an image.  That is not good.

That said, Unlock looks like it should draw its contents in to mOptSurface if it is around which it doesn't currently do since otherwise mOptSurface will be stale.
Attachment #276865 - Flags: review?(pavlov) → review-

Comment 23

11 years ago
Created attachment 277429 [details] [diff] [review]
using LockImageData()/UnlockImageData()

stuart: is this it?

Tested on linux only, doesn't seem lilely to break on other platforms.
Attachment #276865 - Attachment is obsolete: true
Attachment #277429 - Flags: review?(pavlov)

Comment 24

11 years ago
Comment on attachment 277429 [details] [diff] [review]
using LockImageData()/UnlockImageData()

this looks good.
Attachment #277429 - Flags: review?(pavlov) → review+

Comment 25

11 years ago
In UnlockImagePixels, replace:
  if (mImageSurface) {
  if (mImageSurface && mOptSurface) {
Otherwise SetSource could get a null pointer.

this now becomes:
     if (mImageSurface && mOptSurface) {
+        nsRefPtr<gfxContext> context = new gfxContext(mOptSurface);
+        context->SetOperator(gfxContext::OPERATOR_SOURCE);
+        context->SetSource(mImageSurface);
+        context->Paint();
         // Don't need the pixel data anymore
         mImageSurface = nsnull;

Also doing Lock/Unlock for every call to OutputScanlines() can be costly, but it the most simple to force image updating.

Comment 26

11 years ago
they will basically be no-ops for unoptimized images so this will only really slow down OutputScanlines for multipart/x-mixed-replaced images.

Comment 27

11 years ago
Created attachment 277433 [details] [diff] [review]
same with extra null check
Attachment #277429 - Attachment is obsolete: true
Attachment #277433 - Flags: superreview?(pavlov)


11 years ago
Attachment #277433 - Flags: superreview?(pavlov) → superreview+


11 years ago
Keywords: checkin-needed

Comment 28

11 years ago
are you sure you don't need approval first?
Keywords: checkin-needed


11 years ago
Attachment #277433 - Flags: approval1.9?

Comment 29

11 years ago
I have heard that "blockers already plus'd are auto-approved" means that bugs with blocking1.9+ don't need aproval, but I haven't confirmed that.
Comment 29 is correct per the tree rules at the top of .
Keywords: checkin-needed
Assignee: pavlov → asmith15
gfx/src/shared/gfxImageFrame.cpp 1.40
gfx/src/thebes/nsThebesImage.cpp 1.50
modules/libpr0n/decoders/jpeg/nsJPEGDecoder.cpp 1.76
Last Resolved: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Crash Signature: [@ nsJPEGDecoder::OutputScanlines] [@ gdk_rgb_convert_0888] [@ gdk_rgb_init]
You need to log in before you can comment on or make changes to this bug.