greer Reporter
	Description • 24 years ago

This one is showing up in the Linux M09 build (2001050822). Looking at the 
comments it may be related to bug 76936 
(nsPluginInstanceOwner::~nsPluginInstanceOwner)

(30155720) Comments: quitting netscape after a longish sessionnote that
this page has a java applet
(30196733) Comments: I closed netscape done through "file/quit"
(30185093) Comments: I quit Navigator
(30170956) Comments: I just wanted to exit the browser.
(30157054) Comments: I just quit the browser with Ctrl-Q...
(30144960) Comments: closing mozilla
(30142454) Comments: Picked the "Close"-menuitem from the adressbook
windows "file" menu.



nsPluginInstanceOwner::GetInstance()
         nsObjectFrame::Destroy()
         nsLineBox::DeleteLineList()
         nsBlockFrame::Destroy()
         nsFrameList::DestroyFrames()
         nsContainerFrame::Destroy()
         nsFrameList::DestroyFrames()
         nsContainerFrame::Destroy()
         nsFrameList::DestroyFrames()
         nsContainerFrame::Destroy()
         nsFrameList::DestroyFrames()
         nsContainerFrame::Destroy()
         nsTableFrame::Destroy()
         nsFrameList::DestroyFrames()
         nsContainerFrame::Destroy()
         nsTableOuterFrame::Destroy()
         nsLineBox::DeleteLineList()
         nsBlockFrame::Destroy()
         nsFrameList::DestroyFrames()
         nsContainerFrame::Destroy()
         nsFrameList::DestroyFrames()
         nsContainerFrame::Destroy()
         nsFrameList::DestroyFrames()
         nsContainerFrame::Destroy()
         nsFrameList::DestroyFrames()
         nsContainerFrame::Destroy()
         nsTableFrame::Destroy()
         nsFrameList::DestroyFrames()
         nsContainerFrame::Destroy()
         nsTableOuterFrame::Destroy()
         nsLineBox::DeleteLineList()
         nsBlockFrame::Destroy()
         nsLineBox::DeleteLineList()
         nsBlockFrame::Destroy()
         nsFrameList::DestroyFrames()
         nsContainerFrame::Destroy()
         nsFrameList::DestroyFrames()
         nsContainerFrame::Destroy()
         nsBoxFrame::Destroy()
         nsFrameList::DestroyFrames()
         nsContainerFrame::Destroy()
         nsBoxFrame::Destroy()
         nsGfxScrollFrame::Destroy()
         nsFrameList::DestroyFrames()
         nsContainerFrame::Destroy()
         ViewportFrame::Destroy()
         FrameManager::Destroy()
         PresShell::~PresShell()
         PresShell::Release()
         nsCOMPtr_base::~nsCOMPtr_base()
         DocumentViewerImpl::~DocumentViewerImpl()
         DocumentViewerImpl::Release()
         nsCOMPtr_base::assign_with_AddRef()
         nsDocShell::Destroy()
         nsWebShell::Destroy()
         nsDocShell::DestroyChildren()
         nsDocShell::Destroy()
         nsWebShell::Destroy()
         nsXULWindow::Destroy()
         nsWebShellWindow::Destroy()
         nsChromeTreeOwner::Destroy()
         GlobalWindowImpl::Close()
         nsAppShellService::Quit()
         XPTC_InvokeByIndex()
         nsXPCWrappedNativeClass::CallWrappedMethod()
         WrappedNative_CallMethod()
         js_Invoke()
         js_Interpret()
         js_Invoke()
         js_InternalInvoke()
         JS_CallFunctionValue()
         nsJSContext::CallEventHandler()
         nsJSEventListener::HandleEvent()
         nsEventListenerManager::HandleEventSubType()
         nsEventListenerManager::HandleEvent()
         nsXULElement::HandleDOMEvent()
         PresShell::HandleDOMEventWithTarget()
         nsMenuFrame::Execute()
         nsMenuFrame::HandleEvent()
         PresShell::HandleEventInternal()
         PresShell::HandleEvent()
         nsView::HandleEvent()
         nsView::HandleEvent()
         nsView::HandleEvent()
         nsView::HandleEvent()
         nsViewManager::DispatchEvent()
         HandleEvent()
         nsWidget::DispatchEvent()
         nsWidget::DispatchWindowEvent()
         nsWidget::DispatchMouseEvent()
         nsWidget::OnButtonReleaseSignal()
         nsWindow::HandleGDKEvent()
         dispatch_superwin_event()
         handle_gdk_event()
         libgdk-1.2.so.0 + 0x17507 (0x406cc507)
         libglib-1.2.so.0 + 0x10309 (0x406fc309)
         libglib-1.2.so.0 + 0x10913 (0x406fc913)
         libglib-1.2.so.0 + 0x10aac (0x406fcaac)
         libgtk-1.2.so.0 + 0x93c57 (0x40610c57)
         nsAppShell::Run()

	greer Reporter
	Comment 1 • 24 years ago

Adding keywords crash, topcrash, qawanted for tracking

	edburns
	Comment 2 • 24 years ago

Peter, can you please look at this?  I think this is related to bug 73196.

	greer Reporter
	Comment 3 • 24 years ago

edburns, would you please verify that you meant bug 73196 in your previous 
comment. That is a Windows bug related to GIF corruption.

	shrirang khanzode
	Comment 4 • 24 years ago

bug 76936 ?

	Peter Lubczynski
	Comment 5 • 24 years ago

Eeek...where did my last comments go?

Anyway, this is a problem that we try to do an Addref when the vtable looks 
toasted. Another strong ref problem? See bug 79872 for more info. 

	greer Reporter
	Comment 6 • 24 years ago

Here are some extended comments and a few URLs (from this morning's reports):

     (30442798) URL: http://cs.nyu.edu/visual/home/demos/tigerDemo
     (30442798) Comments: I exited the program (the Quit option from the File 
menu) after lookingat a couple of pages with java applets.  The first applet I 
looked at was http://cs.nyu.edu/been/demo/latest.applet.html.  I also had the 
java console open.  I hadn't noticed that there was any problem.

     (30423591) URL: http://www.benchmarkiv.com
     (30423591) Comments: I had a chance to check out the auto resizingof 
animated gifs a little further. Any time that an html height and width 
specification differs from the dimentions inherent in the gif mozilla 9 crops 
the gif to some unspecifiied size.  This happens whether or not the gif is part 
of a javascript.. Thus making this a very serious problem. Also why don't you 
specify in your release notes that only IBMJava2-13 works with
your browser on Redhat 7.1 instead of providing automatic plugin support to 
netscape. Konquer Java works fine with undeprecated glibc libraries. So I'm 
wondering if the problem isJava or your browser. Gene
     
     (30397961) URL: http://www.benchmarkiv.com
     (30397961) Comments: there seems to be a problem with the auto resizing of 
animateid gif's when they are embedded in a javascript. I fixed the problem on 
this site bymaking the gif exactly the right size. IBMJava2-13seems to be 
working fine.

     (30269103) URL: http://www.benchmarkiv.com
     (30269103) Comments: I finally got java to work using the IBM Java2-13. and 
the redhat 7.1jvm workaround. Your java does not work even with their work 
around which makes the kernel use old 2.2 glibc. IBMJava works but someanimated 
gifs only dispay half their area and I get this screen every timeI quit Mozilla. 

     (30284572) Comments: I had actually just closed the browser because it was 
eating up an inordinate amount of CPU cycles and didn't seem to want to stop.   
The only thing out of the ordinary on the page I was viewing was a Java Applet I 
believe.  I'm using the java plugin that came with JDK 1.3.0_01

     (30196733) URL: http://www.dft.nl/ and http://www.detelefoongids.nl/ 
     (30341616) URL: /usr/java/jdk1.3.0_02/demo/jfc/Java2D/Java2Demo.html
     (30185093) URL: http://www.student.carleton.edu/m/meadowa
     (30155720) URL: http://www.rijkswacht.be/rw/verkeer/module.htm

	Peter Lubczynski
	Comment 7 • 24 years ago

nsPluginInstanceOwner::GetInstance() addrefs, that's probably the problem

	chris hofmann
	Comment 8 • 24 years ago

good if we could get this fixed for 0.9.1... is that possible?

	Peter Lubczynski
	Comment 9 • 24 years ago

This bug needs to be re-evaluated with Talkback reports. I think recent
check-ins have fixed this and I can not reproduce but I'm unwilling to close out 
a topcrash bug without any evidence. 

Can someone see if they can kick cyclone into spitting out a report on this 
starting with Friday evening builds?

	chris hofmann
	Comment 10 • 24 years ago

looks like the last time saw incoming reports for 'plugininstanceowner' crashes
was on the 18th....  here are the counts of such crashes

   6 2001051813
   3 2001051812
  21 2001051710
   9 2001051707

would the post May 18 lack of crashes match up with checkin fixes?

	Peter Lubczynski
	Comment 11 • 24 years ago

Yup, thanks! Marking FIXED.

	shrirang khanzode
	Comment 12 • 24 years ago

my stack traces do not show this anymore. Also, based on chofmann's comment, 
marking VERIF.

	David Baron :dbaron:
	Comment 13 • 24 years ago

Reopening.  Today's
http://ftp.mozilla.org/pub/data/crash-data/seamonkey-crash-analysis-detailed.txt
contains:

nsPluginInstanceOwner::GetInstance   5
			 80105 	 VERI 	 FIXE 	 peterlubczynski@netscape.com 	 mozilla0.9.1
     First BBID : http://climate/reports/stackcommentemail.cfm?dynamicBBID=31148923
     Last BBID  : http://climate/reports/stackcommentemail.cfm?dynamicBBID=31201391
     Min Runtime :270
     Max Runtime :124585
     Min seconds since last crash :270
     Max seconds since last crash :82576
     First Appearance Date : 2001-05-31
     Last Appearance Date : 2001-06-01
     First Build ID : 2001053006
     Latest Build ID : 2001060108

Stack Trace: 

    nsPluginInstanceOwner::GetInstance adf24497
	http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/html/base/src/nsObjectFrame.cpp line 1874
	Build: 2001053006 CrashDate: 2001-06-01 UptimeMinutes: 675  Total: 2076 
	OS: Windows NT  4.0 build 1381
	 Detailed : http://climate/reports/incidenttemplate.cfm?bbid=31201391
	 StackTrace: http://climate/reports/stackcommentemail.cfm?dynamicBBID=31201391
     (31201391)	URL: http://www.mlb.com
     (31201391)	Comments: shutting down

    nsPluginInstanceOwner::GetInstance() c4e106db
	 line 
	Build: 2001060108 CrashDate: 2001-06-01 UptimeMinutes: 4  Total: 4 
	OS: Linux 2.4.3
	 Detailed : http://climate/reports/incidenttemplate.cfm?bbid=31195523
	 StackTrace: http://climate/reports/stackcommentemail.cfm?dynamicBBID=31195523

    nsPluginInstanceOwner::GetInstance() 719917b7
	 line 
	Build: 2001053008 CrashDate: 2001-05-31 UptimeMinutes: 71  Total: 71 
	OS: Linux 2.2.16-3
	 Detailed : http://climate/reports/incidenttemplate.cfm?bbid=31172555
	 StackTrace: http://climate/reports/stackcommentemail.cfm?dynamicBBID=31172555
     (31172555)	URL: java.sun.com
     (31172555)	Comments: selected file->exit from drop dowm menu

    nsPluginInstanceOwner::GetInstance() 0d252591
	 line 
	Build: 2001053007 CrashDate: 2001-05-31 UptimeMinutes: 129  Total: 129 
	OS: Linux 2.2.12-20smp
	 Detailed : http://climate/reports/incidenttemplate.cfm?bbid=31157638
	 StackTrace: http://climate/reports/stackcommentemail.cfm?dynamicBBID=31157638
     (31157638)	Comments: Exiting the application

    nsPluginInstanceOwner::GetInstance 16686d73
	http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/html/base/src/nsObjectFrame.cpp line 1874
	Build: 2001053009 CrashDate: 2001-05-31 UptimeMinutes: 1376  Total: 1376 
	OS: Windows NT  4.0 build 1381
	 Detailed : http://climate/reports/incidenttemplate.cfm?bbid=31148923
	 StackTrace: http://climate/reports/stackcommentemail.cfm?dynamicBBID=31148923
     (31148923)	Comments: real crash

	Peter Lubczynski
	Comment 14 • 24 years ago

Looking closer at the crash data, looks like it happens on NT too, and on 
exiting the app. From that info, I think this is a different bug than I fixed 
last time, but I guess we can use this bug as the summary fits.

I've seen this before but I thought it had got fixed or I got it mixed up with 
the "service manager held past XPCOM shutdown/java bug". The problem as I 
remember it was that we were trying to addref a stale pointer when GetInstance 
was called. In other words, the Instance went away too soon.

I *really* need some testcases to reproduce the crash. It would be good to see 
how bad this is as exiting the app while in a simple Java 
(http://www.javasoft.com) or Real testcase seem to work fine for me on Win32. 
Also shuting down while at http://www.mlb.com worked for me as well. I wonder if 
it's something to do with scripting and the instance's refcount as well as 
platform??

If I recall the questionable code correctly, we need to either keep a better 
track of the plugin instance lifetime or (I just started reading about this) use 
something like an nsIWeakRef which will I think should NULL out the dangling 
pointer.

	Peter Lubczynski
	Comment 15 • 24 years ago

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

	selmer (gone)
	Comment 16 • 24 years ago

Is there any "avoid the crash" hack that can go in for m.9.1?  We're almost
completely out of time...

	shrirang khanzode
	Comment 17 • 24 years ago

the steps in bug 83499 is a good testcase to reproduce this crash. I crash 100% 
of the time.

	Peter Lubczynski
	Comment 18 • 24 years ago

Andrei, could you take a look at this if you get the chance?
http://dailynews.yahoo.com/h/a/g/bs/?u

It seems to be doing something similar to the Shockwave registration bug in that 
they do ReloadPlugins() but this time with both XPCOM Java and Real. The 
instance is probably garbage because Shutdown() was called Java and Real when 
doing ReloadPlugins(). It probably has to do with TryUnloadPlugin()

	Marek Z. Jeziorek
	Comment 19 • 24 years ago

is av on vacation still?

	selmer (gone)
	Comment 20 • 24 years ago

Phil, you were going to reassign this or something.

	Phil Peterson
	Comment 21 • 24 years ago

av is back

	av (gone) Assignee
	Comment 22 • 24 years ago

Is my understanding correct that this bug is about XPCOM plugins?

Peter, I am getting a stack trace which may look interesting for you:
EMBD3260! 61508633()
EMBD3260! 6150754a()
NPPL3260! 615549e3()
NPPL3260! 6155dbcd()
nsPluginStreamListenerPeer::SetUpStreamListener(nsIRequest * 0x0645f190, nsIURI 
* 0x063de710) line 1929 + 25 bytes
nsPluginStreamListenerPeer::OnStartRequest(nsPluginStreamListenerPeer * const 
0x06487760, nsIRequest * 0x0645f190, nsISupports * 0x00000000) line 1596 + 21 
bytes
nsHttpChannel::OnStartRequest(nsHttpChannel * const 0x0645f194, nsIRequest * 
0x06409464, nsISupports * 0x00000000) line 2023
nsOnStartRequestEvent::HandleEvent() line 108 + 53 bytes
nsARequestObserverEvent::HandlePLEvent(PLEvent * 0x0641d6f4) line 64
PL_HandleEvent(PLEvent * 0x0641d6f4) line 590 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x00d508d0) line 520 + 9 bytes

	av (gone) Assignee
	Comment 23 • 24 years ago

After removing yahoo cookies (to initiate this checking process) I cannot even 
get the place of supposed crash as I crash earlier. The stack trace follws.

	Peter Lubczynski
	Comment 26 • 24 years ago

Okay, I can now reporduce this easily with the test url. As you can see from the
*complete* stack, the problem happens when there is a
javascript://navigator.plugins.refresh(true); The (true) part is causing the
page to be torn down, we've changed the state of the plugin such that the
instance pointer (mInstance probably went away) is now pointing to garbage,
hence the crash way farther down the road in page destruction, in
::GetInstance() when we try to get the instance to stop it. What is strange is
that the Real player appears and then the crash happens. The console also spits out:
Inside ns4xPluginInstance::SetWindow(00000000)...
ns4xPluginInstance::Stop()

Andrei and Nick: How were XPCOM Plugins designed to deal with a Refresh()?

	av (gone) Assignee
	Comment 27 • 24 years ago

There is no actual difference. It all goes through the same code path 
(nsPluginHostImpl::LoadPlugins) as during start up.

BTW, did you notice that there are three object frames being created: one for 
object tag, one for embed tag and one for applet tag?

	Peter Lubczynski
	Comment 28 • 24 years ago

Ahh...an easier way to reproduce, with a different code path but same crash is
visit ANY Real site, and while the plugin is playing, issue a
javascript.plugins.refresh(1) in the Javascript console.

	Peter Lubczynski
	Comment 29 • 24 years ago

Crashes even with simple testcases on Rrefresh();

http://panther.mcom.com/testcases/plugins/testcases/real1.html

	Peter Lubczynski
	Comment 31 • 24 years ago

This patch seems to sort of help on simple testcases. The big Yahoo one still fails.

Index: nsPluginHostImpl.cpp
===================================================================
RCS file: /cvsroot/mozilla/modules/plugin/nglsrc/nsPluginHostImpl.cpp,v
retrieving revision 1.257.4.1
diff -u -5 -r1.257.4.1 nsPluginHostImpl.cpp
--- nsPluginHostImpl.cpp        2001/06/06 16:37:16     1.257.4.1
+++ nsPluginHostImpl.cpp        2001/06/07 02:21:49
@@ -2140,11 +2140,11 @@
     // if we have currently running plugins we should set a flag not to
     // unload them from memory, see bug #61388
     // and form a list of libs to be unloaded later
     for(nsPluginTag * p = mPlugins; p != nsnull; p = p->mNext)
     {
-      if(IsRunningPlugin(p) && (p->mFlags & NS_PLUGIN_FLAG_OLDSCHOOL))
+      if(IsRunningPlugin(p))
       {
         p->mCanUnloadLibrary = PR_FALSE;
         AddToUnusedLibraryList(p->mLibrary);
       }
     }

	Peter Lubczynski
	Comment 32 • 24 years ago

Something that Andrei just metioned and I confirm is that during a reload of any
kind, the new instance of a plugin is created before the old one is destroyed.
In the case of ReloadPlugins(), we probably want the old document to go away
completely so that the instances will be destroyed then thereby preventing this
crash and others like them. Didn't Hyatt make a change like that?

One idea is in nsPluginArray to save the url, navigate to about:blank, reload
the plugins, and then re-navigate back to old url. It won't be pretty but it's
not like it happens often. But alas, I'm tired, something to try tomorrow. 

	av (gone) Assignee
	Comment 33 • 24 years ago

I have tried the following modification in nsPluginArrayImpl::Refresh and it did 
not help.

	Peter Lubczynski
	Comment 36 • 24 years ago

The above hacked patch seem to work for the simplified testcase. However, the
big Yahoo one doens't crash anymore, but goes in an loop of reloading the
current document forever. 

Notice I'm telling the plugin host to do a normal reload instead of a refresh.

	av (gone) Assignee
	Comment 37 • 24 years ago

If you do pm->ReloadPlugins(PR_FALSE) everything else is probably irrelevant 
because the plugin remains in memory. But this approach will crash the thing if 
the dll is updated. (Will Windows allow us to replace the dll if the plugin is 
playing?)

	av (gone) Assignee
	Comment 38 • 24 years ago

If you do pm->ReloadPlugins(PR_FALSE) everything else is probably irrelevant 
because the plugin remains in memory. But this approach will crash the thing if 
the dll is updated. (Will Windows allow us to replace the dll if the plugin is 
playing?)

	av (gone) Assignee
	Comment 39 • 24 years ago

urgh. Sorry for duplicate comment. Is there a way to edit bug report?

	av (gone) Assignee
	Comment 40 • 24 years ago

I think I figured out a way to set up a web progress listener correctly and now 
can wait for the blank page to load completely and then to refresh the plugin 
list. (It fixes simple case.) Everything goes more or less smooth on the yahoo 
site and I can even see the player after refresh but the it comes to refresh 
again and crashes. I tried to turn off our plugin refresh stuff completely (just 
put return statement in the very beginnig of nsPluginArrayImpl::Refresh(...)) 
and it still crashes! I also crash with 4.x. Starting to get an impression that 
we are fighting ghosts here. Peter, have you ever seen this thing working on any 
browser?

	Peter Lubczynski
	Comment 41 • 24 years ago

You may be right!!! However, all the videos at Yahoo.com work very nicely and
don't crash! We should at least not crash.

Okay, we should fix the simple testcase so it doesn't crash. My changes (which
are similar to yours) prevent the crash in both the simple testcase and the
yahoo one. 

Question: instead of using nsIWebNavigation, could you directly use
nsIDocShell->LoadURI with setting SetStopActiveDocument to PR_FALSE? Anyone more
familiar with WebNav/DocShell code?

However, the Yahoo testcase, is eitehr INVALID or possibly a dup of a Java bug.
Andrei, check out the source, they are using document.write to try to
create an instance of this Java class. Opening the Java console (be sure you
have java installed) reveals an exception and probably the reason my patch ended
up in a loop:

java.lang.NoClassDefFoundError: RMObserver

	at java.lang.ClassLoader.defineClass0(Native Method)

	at java.lang.ClassLoader.defineClass(Unknown Source)

	at java.security.SecureClassLoader.defineClass(Unknown Source)

	at sun.applet.AppletClassLoader.findClass(Unknown Source)

	at sun.plugin.security.PluginClassLoader.findClass(Unknown Source)

	at java.lang.ClassLoader.loadClass(Unknown Source)

	at sun.applet.AppletClassLoader.loadClass(Unknown Source)

	at java.lang.ClassLoader.loadClass(Unknown Source)

	at sun.applet.AppletClassLoader.loadCode(Unknown Source)

	at sun.applet.AppletPanel.createApplet(Unknown Source)

	at sun.plugin.AppletViewer.createApplet(Unknown Source)

	at sun.applet.AppletPanel.runLoader(Unknown Source)

	at sun.applet.AppletPanel.run(Unknown Source)

        at java.lang.Thread.run(Unknown Source)

	Peter Lubczynski
	Comment 42 • 24 years ago

To clarify, I do not crash in 4.x, it works nicely.

	av (gone) Assignee
	Comment 44 • 24 years ago

No, doesn't seem to help with simplified test case.

Just to confirm, hitting Reload button works fine, as well as similar test case 
with QT. So the problem is only with xpcom pluigns on 
javascript:navigator.plugins.refresh(1) command.

	David Hyatt
	Comment 45 • 24 years ago

Can you send me a link or pointer to the QT test case?

Thanks!

	av (gone) Assignee
	Comment 47 • 24 years ago

And please also note that QT test worked before applying the latest patch too. 
As well as simple Reload.

	David Hyatt
	Comment 48 • 24 years ago

Ok, I'm trying something else.  Will post a new patch momentarily.

	David Hyatt
	Comment 49 • 24 years ago

I cannot stop this crash.  I've written a lot of code now, and I'm fairly
certain this is not my bug.

I can tell you this for sure:
  (1) The crash is happening in the new document after painting has been
unsuppressed.
  (2) Even if I synchronously destroy the old document before the reload from
the refresh even happens, the new document crashes in the same place.
  (3) If i add back the original code that nulls out the content viewer
immediately, i.e., completely remove the hack to keep the old page alive, I
*still* crash in the same place.

This bug does not appear IMO to have anything to do with my patch.  I would
theorize that a leak of the old document's presshell is perhaps keeping the old
frame tree alive.

I'm going to back away from this bug now, although I might incidentally fix this
bug when I tackle 80203.  My fix for 80203 will force a presshell (even if
leaked) to destroy its frame tree.  That is not currently targeted at RTM,
however, although, if it fixes this bug, perhaps it should be.

	Peter Lubczynski
	Comment 50 • 24 years ago

Hyatt,

I've tried your patch on the simple testcase:  
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=37463

and things seem to be a bit better. I don't crash. And it appears to function
corectly, even playing. However, loading up the big testcase at Yahoo! News
comes up blank with a bunch of style errors in the console. On a Reload, more
frame "aCombinedWidth" ASSERTIONS until a final crash while trying to set up a
new stream with a dangling instance pointer.

However, one thing I did notice was that frame destruction/creation seemed not
to be effected. Put printf's in the destructor and constructor of
nsPluginInstanceOwner in nsObjectFrame.cpp. This has an ownening reference to
the instance and notice that still a new one is creted before the old one is
destroyed.

	av (gone) Assignee
	Comment 51 • 24 years ago

With recent build I cannot get to the point it crashes on the Yahoo site -- 
blank window appears doing nothing. Simlified test case still crashes. Changing 
summary to make it more informative. Setting eta.

Just not to loose, the original Yahoo URL was 
http://dailynews.yahoo.com/h/a/g/bs/?u

	Peter Lubczynski
	Comment 52 • 24 years ago

Andrei, this patch may help as it prevents the simple testcase from crashing:
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=37658

Weird, though, about:plugins goes blank!

	av (gone) Assignee
	Comment 53 • 24 years ago

I also have a patch to avoid a crash in the simplified case. Will investigate 
tomorrow which one is better. Or will come up with yet another one.

	Jan Carpenter
	Comment 54 • 24 years ago

M091 topcrash. Adding  [@ nsPluginInstanceOwner::GetInstance] for tracking

Here are comments/urls from recent crashes:

   (31750805)
Comments: I selected File/Close while I had 6 open browser
windows.
     (31738479) URL: www.eet.com
     (31738479) Comments: running from an exceed session. exceed
also seem to be having problems
     (31737630) Comments: Exiting Mozilla after leaving it on for
more than 3 days browsing a lot of pages in the meantime.
     (31725369) URL: sportsoul.tiscalinet.it
     (31720639) Comments: closed it
     (31718843) Comments: loading yahoo realvideo site
     (31714158) Comments: crashed when loading Font2DTest.html
from Java 1.3.1 distribution.
     (31700433) Comments: closing (with ^q) - it had closed 5 or 6
of the 6 windows i had open before it crashed
     (31694583) Comments: quitting
     (31690418) Comments: Closing the addressbook window.  I had
navigator and mail open but mail was unable to reach my POP server
due to firewall.
     (31689866) Comments: File->Quit
     (31685992) Comments: selected "File/Quit" to quit browser
suite after listening to Netscape Radio radio.netscape.com
     (31682916) Comments: Actually
     (31677080) Comments: Quality Feedback Agent appears when
closing mozilla via File->Quit
     (31676345) URL: http://www.broadcast.com
     (31676345) Comments: New Broadcast tests passed
     (31658032) URL: http://www.saferinternet.org/
     (31658032) Comments: closing mozilla from this page
     (31624122) Comments: Closed Mozilla with Ctrl-Q. Had Java
Applet running. Copied Realplayer plugin files to mozilla/plugins
directory while mozilla was running.Maybe I shouldn't have done
that?
     (31621788) Comments: quitting...


Here is a stack trace from the 2001060713 build

Incident ID 31752015 
nsPluginInstanceOwner::GetInstance() 
nsObjectFrame::Destroy() 
nsLineBox::DeleteLineList() 
nsBlockFrame::Destroy() 
nsLineBox::DeleteLineList() 
nsBlockFrame::Destroy() 
nsFrameList::DestroyFrames() 
nsContainerFrame::Destroy() 
nsFrameList::DestroyFrames() 
nsContainerFrame::Destroy() 
nsBoxFrame::Destroy() 
nsFrameList::DestroyFrames() 
nsContainerFrame::Destroy() 
nsBoxFrame::Destroy() 
nsGfxScrollFrame::Destroy() 
nsFrameList::DestroyFrames() 
nsContainerFrame::Destroy() 
ViewportFrame::Destroy() 
FrameManager::Destroy() 
PresShell::~PresShell() 
PresShell::Release() 
nsCOMPtr_base::~nsCOMPtr_base() 
DocumentViewerImpl::~DocumentViewerImpl() 
DocumentViewerImpl::Release() 
nsCOMPtr_base::assign_with_AddRef() 
nsDocShell::Destroy() 
nsWebShell::Destroy() 
nsHTMLFrameInnerFrame::~nsHTMLFrameInnerFrame() 
nsFrame::Destroy() 
nsFrameList::DestroyFrames() 
nsContainerFrame::Destroy() 
nsFrameList::DestroyFrames() 
nsContainerFrame::Destroy() 
nsBoxFrame::Destroy() 
nsFrameList::DestroyFrames() 
nsContainerFrame::Destroy() 
nsBoxFrame::Destroy() 
nsFrameList::DestroyFrames() 
nsContainerFrame::Destroy() 
nsBoxFrame::Destroy() 
nsFrameList::DestroyFrames() 
nsContainerFrame::Destroy() 
nsBoxFrame::Destroy() 
nsFrameList::DestroyFrames() 
nsContainerFrame::Destroy() 
nsBoxFrame::Destroy() 
nsFrameList::DestroyFrames() 
nsContainerFrame::Destroy() 
ViewportFrame::Destroy() 
FrameManager::Destroy() 
PresShell::~PresShell() 
PresShell::Release() 
nsCOMPtr_base::~nsCOMPtr_base() 
nsView::HandleEvent() 
nsViewManager::DispatchEvent() 
HandleEvent() 
nsWidget::DispatchEvent() 
nsWidget::DispatchWindowEvent() 
nsWidget::DispatchMouseEvent() 
nsWidget::OnButtonReleaseSignal() 
nsWindow::HandleGDKEvent() 
dispatch_superwin_event() 
handle_gdk_event() 
libgdk-1.2.so.0 + 0x1753b (0x4031653b) 
libglib-1.2.so.0 + 0x10186 (0x40346186) 
libglib-1.2.so.0 + 0x10751 (0x40346751) 
libglib-1.2.so.0 + 0x108f1 (0x403468f1) 
libgtk-1.2.so.0 + 0x8c8e9 (0x4026a8e9) 
nsAppShell::Run() 
nsAppShellService::Run() 
main1() 
main() 
libc.so.6 + 0x189cb (0x4043f9cb) 

	Peter Lubczynski
	Comment 55 • 24 years ago

I don't think that the talkback report is for this bug or this bug has morphed
as people's comments in the report indicate, the crash in ::GetInstance() is a
more general probelm than just with navigator.plugins.refresh(1). Perhaps we
should open a new bug?

Andrei, as a hack to fix the more general problem, perhaps we could publicly
expose nsActivePluginList in such a way that InstanceOwner can  get at the them
and check the "stopped" state or even if the instance with the stale pointer is
in the list. I can see this "bullet proofing" here and in other places where we
need to know the active plugin instances but I don't like the idea that this is
needed because the "instance" went away without it's "instance owner"!!!!

	av (gone) Assignee
	Comment 56 • 24 years ago

Peter, the fact that the patch 
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=37658
help us prevent crash has nothing to do with redirection to another page first. 
It is all about doing ReloadPlugins(PR_FALSE) rather than ReloadPlugind(1), so 
the simplest patch with the same effect would be just this one-liner:

 nsCOMPtr<nsIPluginManager> pm(do_QueryInterface(mPluginHost));

 if(pm)
-  pm->ReloadPlugins(aReloadDocuments);
+  pm->ReloadPlugins(PR_FALSE);

 if (aReloadDocuments && mDocShell) {
   nsCOMPtr<nsIWebNavigation> webNav(do_QueryInterface(mDocShell));

   if (webNav)
     webNav->Reload(nsIWebNavigation::LOAD_FLAGS_NONE);
 }

	av (gone) Assignee
	Comment 58 • 24 years ago

The patch above just notifies the PluginInstanceOwner that the plugin instance 
has been destroyed by setting its member variable mInstance to null. Note that 
nsPluginInstanceOwner destructor was not bulletproofed againt the situation when 
mInstance in null.

	Peter Lubczynski
	Comment 59 • 24 years ago

wonderful! r=peterl

	Marc Attinasi
	Comment 60 • 24 years ago

sr=attinasi

	Robert O'Callahan (:roc) (email my personal email if necessary)
	Comment 61 • 24 years ago

a=roc on behalf of drivers

	av (gone) Assignee
	Comment 62 • 24 years ago

Checked in. Marking fixed.

	Peter Lubczynski
	Comment 63 • 24 years ago

Reopening. Andrei, you forgot to add 

+    NS_IF_RELEASE(mInstance);

to nsPluginViewer.cpp. This causes a crash.

	Peter Lubczynski
	Comment 64 • 24 years ago

Since I'm touching that file in bug 59749 anyway, I've included the fix in
there. Take or leave the new interface ;) Here's the patch:

http://bugzilla.mozilla.org/showattachment.cgi?attach_id=39274

	av (gone) Assignee
	Comment 65 • 24 years ago

Ah! My bad. Do you think you will check 59749 in soon or I should push this 
one-liner? Thanks for the catch!

	Peter Lubczynski
	Comment 66 • 24 years ago

Either way is fine with me. Both need to go through the review/approval process,
but a one liner is quicker than a new interface with a cross module patch.. That
last patch fixs the bug with no side effects (that I have seen) except we now
have a public nsIPluginViewer interface (in IDL too!) which currently only
serves the one simple task of cleaning the titlebar from the docshell. We kind
of needed that anyway, and can add functionality like scripting to it later.

Could your review that patch?

	av (gone) Assignee
	Comment 67 • 24 years ago

Patch to fix the remaining issue:

Index: nsPluginViewer.cpp
===================================================================
RCS file: /cvsroot/mozilla/modules/plugin/nglsrc/nsPluginViewer.cpp,v
retrieving revision 1.58
diff -u -r1.58 nsPluginViewer.cpp
--- nsPluginViewer.cpp  2001/06/18 21:41:02     1.58
+++ nsPluginViewer.cpp  2001/06/20 21:52:41
@@ -951,7 +951,7 @@
       if(host)
         host->StopPluginInstance(mInstance);
     }
-    NS_RELEASE(mInstance);
+    NS_IF_RELEASE(mInstance);
   }

   mWindow = nsnull;

	av (gone) Assignee
	Comment 68 • 24 years ago

We should stop this crash as soon as possible. Please review and approve.

	Peter Lubczynski
	Comment 69 • 24 years ago

r=peterl

	Marc Attinasi
	Comment 70 • 24 years ago

sr=attinasi - since it was in the original patch, but you forgot to check that
part in, I'd say that the original r/sr/a all apply (but that is just my opinion)

	chris hofmann
	Comment 71 • 24 years ago

a-chofmann

	Peter Lubczynski
	Comment 72 • 24 years ago

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

	av (gone) Assignee
	Comment 73 • 24 years ago

Checked in. ==> fixed.

	Christopher Blizzard (:blizzard)
	Comment 74 • 24 years ago

...just tracking...

	shrirang khanzode
	Comment 75 • 24 years ago

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

	shrirang khanzode
	Comment 76 • 24 years ago

crash not happening anymore on today's win 0621 trunk. verif all dups...

	varada
	Comment 77 • 24 years ago

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