154427 - flash crashes on local display :0.1

Status: VERIFIED WORKSFORME

(External Software Affecting Firefox Graveyard :: Flash (Adobe), defect, P2)

Description

[Jamie Zawinski]

Since bug 58937 describes two seperate problems, I'm entering this bug to divide
them.

Bug 58937 describes the problem that Flash crashes when $DISPLAY is set to a
non-local host, due to XSHM bugs in the Flash plugin itself.  That problem can
likely only be fixed by Macromedia.

*This* bug describes a bug in Mozilla that causes flash to crash when $DISPLAY
is anything but :0.0.  That is, on a multi-head machine, the Flash plugin causes
Mozilla to crash if Mozilla is being run on any screen except screen 0.

This is absolutely, without question a bug in MOZILLA, not in FLASH.

This is easily demonstrated by the fact that the very same Flash plugin works
fine in Netscape 4.78, but causes Mozilla to crash.

The nature of this bug is almost certainly that Mozilla is handing the Flash
plugin the wrong Screen and/or Visual (probably always using screen 0 instead of
DefaultVisual.)

Netscape 4.78 did not have that bug, and so the Flash plugin does not crash in
Netscape.

Comment 1

[rubydoo123]

handing over to serge

Comment 2

[Jamie Zawinski]

In bug 58937, Roland Mainz wrote:
>
> If someone is interested in getting 33.33333% of this bug fixed then please file
> a bug "Mozilla plugin API passes the wrong visual to plugin when screen's
> default visual!=Mozilla's window visual" and assign it to me...

Comment 3

[Roland Mainz]

Jamie Zawinski wrote:
> If someone is interested in getting 33.33333% of this bug fixed then
> please file
> a bug "Mozilla plugin API passes the wrong visual to plugin when screen's
> default visual!=Mozilla's window visual" and assign it to me...

Wrong bug... ;-(
Passing the wrong viusal to the Xt plugin "root" widget is not the same bug as
this one.

Comment 4

[Jamie Zawinski]

It's not?  What do you think this bug says if not "the plugin is being invoked
with a screen and/or visual that is incompatible with the window"?

Comment 5

[Roland Mainz]

Jamie Zawinski wrote:
> It's not?  What do you think this bug says if not "the plugin is being invoked
> with a screen and/or visual that is incompatible with the window"?

Well, the summary made me think that this bug is dualhead only while the issue I
am thinking about _always_ occurs if - for example - the screen's default visual
is a 8bit PseudoColor one but the same screen has a 24bit TrueColor visual, too.
Then the libGDK code fetches the 24bit TrueColor visual and all our windows use
it then - but we do not tell the Xt widget machinery about that choice...
well... and then it bites back... ;-(

Comment 6

[Jamie Zawinski]

Well, I'm betting if you fix your bug, my bug will be fixed as a result!  I'm
guessing the same line of code is at fault in both cases.

Comment 7

[serge (gone)]

>but we do not tell the Xt widget machinery about that choice
didn't pachces for bug 85958 fixed that?

Comment 8

[Roland Mainz]

[e-mail] wrote:
> > but we do not tell the Xt widget machinery about that choice
> didn't pachces for bug 85958 fixed that?

Oh. Yes... the first patch fixes that issue... however you are not patching the
Xlib toolkit (bad) ... ;-(((((((((((

Comment 9

[serge (gone)]

I thought xxlib_rgb_get_visual() does the right thing 
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/widget/src/xlibxtbin/xlibxtbin.cpp&rev=1.3&root=/cvsroot#91

Comment 10

[Roland Mainz]

[e-mail] wrote:
> I thought xxlib_rgb_get_visual() does the right thing 
>
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/widget/src/xlibxtbin/xlibxtbin.cpp&rev=1.3&root=/cvsroot#91

Yes and no.
You still have to tell the Xt widgets which visual they should use.

Should I write the patch for that ?

Comment 11

[Jamie Zawinski]

I don't know this code, but it sure looks to me like

    top_widget = XtAppCreateShell("drawingArea", "Wrapper",
                                  applicationShellWidgetClass, xtdisplay,
                                  NULL, 0);

ought to be called with an arglist containing XtNvisual, XtNdepth, and
XtNcolormap.  (And possibly also XtNscreen, but I'm not sure if that's
strictly necessary.)

I think that these are parameters that have to be specified at widget
creation time, not afterward with XSet.  (Though *possibly* you can
still set them up until realization time -- I'm not sure.)

Comment 12

[serge (gone)]

Roland Mainz wrote:
>Should I write the patch for that ?
Sure, that would be great.

Jamie: I've tried to pass all args into XtAppCreateShell(), it didn't help,
flash crashed with the same stack trace:(

Comment 13

[serge (gone)]

*** Bug 135655 has been marked as a duplicate of this bug. ***

Comment 14

[rubydoo123]

*** Bug 152884 has been marked as a duplicate of this bug. ***

Comment 15

[Robert Heller]

There is an *important* thought:

If a plugin crashes (for *any* reason), is it reasonable for Mozilla to crash too?

In other words, shouldn't Mozilla protect itself from broken plugins?  Either by
catching crash signals or by running the plugin in a separate process.  Assuming
that all plugins will always behave is an unreasonable assumption.  A *better*
assumption would be that a plugin can crash at any time for any reason. Mozilla
should 'armorplate' itself from a possibly broken plugin.  Esp. since Mozilla
has no real control over the behaviour of the plugin.

Comment 16

[rubydoo123]

Hi Robert: yes it is a very important issue, and we are working at trying to
separate the plug-ins via different thread or a different process (please see
bug 156493).

Comment 17

[Jamie Zawinski]

beppe, what is your malfunction?  have you actually READ this bug?
How can you possibly claim this is a bug in Flash?

IT IS NOT.

IT IS A BUG IN MOZILLA.

Would using smaller words help?

If you believe this is a bug in Flash, then prove it -- I believe
my comments in the initial report of this bug prove beyond any doubt
that this is a bug in Mozilla.

If you believe otherwise, then REFUTE THEM.  Just saying this is a
bug in Flash does not make it so.

Comment 18

[rubydoo123]

Jamie: I'm sorry you disagree with us, but the plug-in team has reviewed the
complaint in this bug and have looked into the issue. Based on the results of
the testing and debugging, it was determined that this is an issue resulting
from the plug-in vendor and not from plug-in code within mozilla.

Please note that we have cc'ed macromedia personnel and that we are working with
them to resolve this issue. We will continue to update the bug as we jointly
investigate with the engineers from macromedia.

Thank you for your interest in this issue. Once it is resolved, we would
appreciate your help in verifying the fix.

Comment 19

[Roland Mainz]

Just wondering:
Do you set |XtNscreen| for the toplevel shell ?
If "yes" - then it is likely that this is a flash plugin issue.
If "no" - then it is likely that this is a mozilla issue.

Comment 20

[Jamie Zawinski]

> Jamie: I'm sorry you disagree with us, but the plug-in team has reviewed the
> complaint in this bug and have looked into the issue. Based on the results of
> the testing and debugging, it was determined that this is an issue resulting
> from the plug-in vendor and not from plug-in code within mozilla.

I don't believe you.

You *might* be right -- but you've given me no reason to think that.

Please make me believe you by actually *explaining* what you think the
problem instead of just giving me a marketroid brush-off.

Please explain how this can possibly be a bug in Flash, given the facts
I stated in this bug report.

If you so sure, that should be easy for you.

Comment 21

[serge (gone)]

Jamie, have you actually READ this bug? gtk visual & colormap problem is fixed
in bug 85958 and your description: "...the Flash plugin causes Mozilla to crash
if Mozilla is being run on any screen except screen 0" is wrong, read bug
152884, or setenv DISPLAY 127.0.0.1:0 and you will crash with exact the same
stack trace:
(gdb) bt
#0 __libc_free (mem=0x4325f008) at malloc.c:3136
#1 0x43137215 in nsMathMLAtoms::ident_ () from
/u/serge/plugins/linux/libflashplayer5.so
#2  0x423500b9 in nsObjectFrame::Destroy (this=0x87c50a8,
aPresContext=0x86a8000) at nsObjectFrame.cpp:695
#3  0x4249017b in nsFrameList::DestroyFrames (this=0x867a324,
aPresContext=0x86a8000) at nsFrameList.cpp:130
#4  0x423113ac in nsContainerFrame::Destroy (this=0x867a2f0,
aPresContext=0x86a8000) at nsContainerFrame.cpp:140
#5  0x42345e4f in nsLineBox::DeleteLineList (aPresContext=0x86a8000,
aLines=@0x8679e30) at nsLineBox.cpp:311
#6  0x422f9d83 in nsBlockFrame::Destroy (this=0x8679df4, aPresContext=0x86a8000)
at nsBlockFrame.cpp:421
#7  0x422f7bf5 in nsAreaFrame::Destroy (this=0x8679df4, aPresContext=0x86a8000)
at nsAreaFrame.cpp:167
#8  0x4249017b in nsFrameList::DestroyFrames (this=0x867984c,
aPresContext=0x86a8000) at nsFrameList.cpp:130
#9  0x422f739e in nsAbsoluteContainingBlock::DestroyFrames (this=0x867984c,
aDelegatingFrame=0x8679800, aPresContext=0x86a8000) at
nsAbsoluteContainingBlock.cpp:385
#10 0x422f9d14 in nsBlockFrame::Destroy (this=0x8679800, aPresContext=0x86a8000)
at nsBlockFrame.cpp:411
#11 0x422f7bf5 in nsAreaFrame::Destroy (this=0x8679800, aPresContext=0x86a8000)
at nsAreaFrame.cpp:167
#12 0x4249017b in nsFrameList::DestroyFrames (this=0x84ef65c,
aPresContext=0x86a8000) at nsFrameList.cpp:130

I believe flash corrupts the mem and crashes:(
can you prove that mozilla corrupts the memory and flash is just a victim?
To prove my believing I refer you to bug 149336 flash corruption of stack 

Comment 22

[Roland Mainz]

OK, after looking at the source:
You do not set |XtNscreen| - I am sure that some plugins will go mad in this
case.

Should I file a patch for that issue ?

Comment 23

[serge (gone)]

sure, lets try to set |XtNscreen|

Comment 24

[Roland Mainz]

Should I take the bug ?

Comment 25

[serge (gone)]

you got it

Comment 26

[Robert Heller]

I think the confussion about whether this is a flash bug or a Mozilla stems from
the fact that there are reall *two* bugs:

There are things wrong with the flash plugin.  Possibly *several* things wrong.
 All of which are in fact Macromedia's 'problem'.

HOWEVER,

There is an *additional* problem:  when a plugin crashes, *IT SHOULD NOT TAKE
THE BROWSER WITH IT*.  The mere fact that Mozilla is crashing is a *MOZILLA*
bug.  It does not matter that the *original* bug happens to be deep in the heart
of some random plugin!!!  Note: this bug will still be here, EVEN WHEN
MACROMEDIA FIXES THE FLASH PLUGIN.  That is, even with a fixed plugin (that does
not crash), the bug in Mozilla will in fact still be there, and will lie
'dormant', until the *next* broken plugin comes along.  At which point we will
have *exactly* the same discussion all over again!

Comment 27

[serge (gone)]

Robert: you are absolutely right, and there is a plan to separate plugin process
from mozilla one.
Roland: I'm getting the same stack trace with this patch:
RCS file: /cvsroot/mozilla/widget/src/gtkxtbin/gtkxtbin.c,v
retrieving revision 1.14
diff -u -r1.14 gtkxtbin.c
--- gtkxtbin.c	20 Jun 2002 01:50:52 -0000	1.14
+++ gtkxtbin.c	24 Jul 2002 01:00:59 -0000
@@ -292,9 +292,14 @@
    * an ugly hack.
    */
 
-  top_widget = XtAppCreateShell("drawingArea", "Wrapper", 
-                                applicationShellWidgetClass, xtbin->xtdisplay, 
-                                NULL, 0);
+  n = 0;
+  XtSetArg(args[n], XtNvisual, GDK_VISUAL_XVISUAL(gdk_window_get_visual(
xtbin->parent_window )) ); n++;
+  XtSetArg(args[n], XtNdepth, gdk_window_get_visual( xtbin->parent_window
)->depth ); n++;
+  XtSetArg(args[n], XtNcolormap,
GDK_COLORMAP_XCOLORMAP(gdk_window_get_colormap( xtbin->parent_window)) ); n++;
+  XtSetArg(args[n], XtNscreen, DefaultScreenOfDisplay(xtbin->xtdisplay)); n++;
+  top_widget = XtAppCreateShell("drawingArea", "Wrapper",
+                                applicationShellWidgetClass, xtbin->xtdisplay,
+                                args, n);

Comment 28

[Roland Mainz]

serge wrote:
> Roland: I'm getting the same stack trace with this patch:
+  XtSetArg(args[n], XtNscreen, DefaultScreenOfDisplay(xtbin->xtdisplay)); n++;

AFAIK the use of |DefaultScreenOfDisplay()| is wrong here.
I'll file a patch for both GTK++Xlib plugin code when I am awake again (e.g.
tomorrow).

Comment 29

[rubydoo123]

Robert: one of the issues you mention, about keeping the app alive even if the
plug-in fails is being tracked in bug 156493

Comment 30

[serge (gone)]

*** Bug 159276 has been marked as a duplicate of this bug. ***

Comment 31

[Jamie Zawinski]

Here's me dying of shock: Flash seems to be working fine with Mozilla 1.2b and
the Flash 6.0 r60 beta from
http://www.macromedia.com/software/flashplayer/special/beta/ on my :0.1 display
on Red Hat 7.2...

Comment 32

[rubydoo123]

so Jamie, since it now works with a Flash update, (notice that was a Flash fix
:o) and not a npapi fix) can we now mark as fixed?

Comment 33

[Jamie Zawinski]

worksforme...

Comment 34

[shrirang khanzode]

wfm

Comment 35

[shrirang khanzode]

.

Comment 36

[BMO Automation]

Version and milestone values are being reset to defaults as part of product refactoring.