Closed
Bug 90951
Opened 23 years ago
Closed 23 years ago
Trunk & N610 crash in [@ 0x00000000 - MimeTypeElementImpl::GetEnabledPlugin] after plugin.refresh()
Categories
(Core Graveyard :: Plug-ins, defect, P1)
Core Graveyard
Plug-ins
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla0.9.3
People
(Reporter: peterlubczynski-bugs, Assigned: peterlubczynski-bugs)
References
()
Details
(Keywords: crash, topcrash, Whiteboard: PDT+)
Crash Data
Attachments
(3 files)
1.22 KB,
text/html
|
Details | |
7.49 KB,
patch
|
Details | Diff | Splinter Review | |
8.45 KB,
patch
|
Details | Diff | Splinter Review |
Click reload while on this page (or the page it redirects to). Notice the crash. Looks like something in the MIME-type array, the plugin's address looks invalid.
Assignee | ||
Comment 1•23 years ago
|
||
ns_if_addref(nsIDOMPlugin * 0x037bba88) line 1129 + 15 bytes MimeTypeElementImpl::GetEnabledPlugin(MimeTypeElementImpl * const 0x037bc528, nsIDOMPlugin * * 0x0012de34) line 217 + 21 bytes XPTC_InvokeByIndex(nsISupports * 0x037bc528, unsigned int 4, unsigned int 1, nsXPTCVariant * 0x0012de34) line 139 XPCWrappedNative::CallMethod(XPCCallContext & {...}, XPCWrappedNative::CallMode CALL_GETTER) line 1881 + 42 bytes XPCWrappedNative::GetAttribute(XPCCallContext & {...}) line 1771 + 14 bytes XPC_WN_GetterSetter(JSContext * 0x03283138, JSObject * 0x0354db78, unsigned int 0, long * 0x0368a178, long * 0x0012e07c) line 1284 + 9 bytes js_Invoke(JSContext * 0x03283138, unsigned int 0, unsigned int 2) line 807 + 23 bytes js_InternalInvoke(JSContext * 0x03283138, JSObject * 0x0354db78, long 56825000, unsigned int 0, unsigned int 0, long * 0x00000000, long * 0x0012edd4) line 896 + 20 bytes js_GetProperty(JSContext * 0x03283138, JSObject * 0x0354db78, long 56342688, long * 0x0012edd4) line 2408 + 45 bytes js_Interpret(JSContext * 0x03283138, long * 0x0012f000) line 2535 + 1998 bytes js_Execute(JSContext * 0x03283138, JSObject * 0x03316890, JSScript * 0x036ffdd0, JSStackFrame * 0x00000000, unsigned int 0, long * 0x0012f000) line 986 + 13 bytes JS_EvaluateUCScriptForPrincipals(JSContext * 0x03283138, JSObject * 0x03316890, JSPrincipals * 0x036970d8, const unsigned short * 0x0012f160, unsigned int 47, const char * 0x036ffbf8, unsigned int 81, long * 0x0012f000) line 3273 + 25 bytes nsJSContext::EvaluateString(nsJSContext * const 0x03311518, const nsAString & {...}, void * 0x03316890, nsIPrincipal * 0x036970d4, const char * 0x036ffbf8, unsigned int 81, const char * 0x01008688, nsAString & {...}, int * 0x0012f06c) line 608 + 85 bytes nsScriptLoader::EvaluateScript(nsScriptLoadRequest * 0x0398ee38, const nsAFlatString & {...}) line 565 nsScriptLoader::ProcessRequest(nsScriptLoadRequest * 0x0398ee38) line 477 + 22 bytes nsScriptLoader::ProcessScriptElement(nsScriptLoader * const 0x036c05e8, nsIDOMHTMLScriptElement * 0x036d4388, nsIScriptLoaderObserver * 0x036d438c) line 420 + 15 bytes nsHTMLScriptElement::SetDocument(nsHTMLScriptElement * const 0x036d4360, nsIDocument * 0x03921d00, int 0, int 1) line 145 nsGenericHTMLContainerElement::AppendChildTo(nsGenericHTMLContainerElement * const 0x036e6d80, nsIContent * 0x036d4360, int 0, int 0) line 3788 HTMLContentSink::ProcessSCRIPTTag(const nsIParserNode & {...}) line 5010 HTMLContentSink::AddLeaf(HTMLContentSink * const 0x036c0470, const nsIParserNode & {...}) line 3435 + 12 bytes CNavDTD::AddLeaf(const nsIParserNode * 0x03731188) line 3789 + 22 bytes CNavDTD::HandleScriptToken(const nsIParserNode * 0x03731188) line 2259 + 12 bytes CNavDTD::OpenContainer(const nsCParserNode * 0x03731188, nsHTMLTag eHTMLTag_script, int 1, nsEntryStack * 0x00000000) line 3447 + 12 bytes CNavDTD::HandleDefaultStartToken(CToken * 0x036d3128, nsHTMLTag eHTMLTag_script, nsCParserNode * 0x03731188) line 1337 + 20 bytes CNavDTD::HandleStartToken(CToken * 0x036d3128) line 1746 + 22 bytes CNavDTD::HandleToken(CNavDTD * const 0x036991e0, CToken * 0x00000000, nsIParser * 0x0264f018) line 910 + 12 bytes CNavDTD::BuildModel(CNavDTD * const 0x036991e0, nsIParser * 0x0264f018, nsITokenizer * 0x0364e5d8, nsITokenObserver * 0x00000000, nsIContentSink * 0x036c0470) line 540 + 20 bytes nsParser::BuildModel() line 2210 + 34 bytes nsParser::ResumeParse(int 1, int 0) line 2081 + 11 bytes nsParser::OnDataAvailable(nsParser * const 0x0264f020, nsIRequest * 0x036e0df0, nsISupports * 0x00000000, nsIInputStream * 0x0364bc28, unsigned int 16687, unsigned int 13764) line 2684 + 19 bytes nsDocumentOpenInfo::OnDataAvailable(nsDocumentOpenInfo * const 0x036e0490, nsIRequest * 0x036e0df0, nsISupports * 0x00000000, nsIInputStream * 0x0364bc28, unsigned int 16687, unsigned int 13764) line 235 + 46 bytes nsStreamListenerTee::OnDataAvailable(nsStreamListenerTee * const 0x03670ff8, nsIRequest * 0x036e0df0, nsISupports * 0x00000000, nsIInputStream * 0x0369ad48, unsigned int 16687, unsigned int 13764) line 56 + 51 bytes nsHttpChannel::OnDataAvailable(nsHttpChannel * const 0x036e0df4, nsIRequest * 0x0369af40, nsISupports * 0x00000000, nsIInputStream * 0x0369ad48, unsigned int 16687, unsigned int 13764) line 2146 + 57 bytes nsOnDataAvailableEvent::HandleEvent() line 175 + 70 bytes nsARequestObserverEvent::HandlePLEvent(PLEvent * 0x036fbdf4) line 64 PL_HandleEvent(PLEvent * 0x036fbdf4) line 590 + 10 bytes PL_ProcessPendingEvents(PLEventQueue * 0x00d3c618) line 520 + 9 bytes _md_EventReceiverProc(HWND__ * 0x000b054a, unsigned int 49482, unsigned int 0, long 13878808) line 1071 + 9 bytes USER32! 77e12e98() USER32! 77e130e0() USER32! 77e15824() nsAppShellService::Run(nsAppShellService * const 0x00df7558) line 422 main1(int 3, char * * 0x00358010, nsISupports * 0x00000000) line 1174 + 32 bytes main(int 3, char * * 0x00358010) line 1478 + 37 bytes mainCRTStartup() line 338 + 17 bytes
Comment 2•23 years ago
|
||
I thought someone on the install team did the plugins.refresh code?
Comment 3•23 years ago
|
||
This bug is also in Bugscape http://bugscape/show_bug.cgi?id=7003
Peter, how is this page different (in whatever matters) from our regular about:plugins page which also does plugins.refresh() but doesn't crash on reload?
Comment 6•23 years ago
|
||
Bug 91569 has another testcase for this involving about:plugins. And, lots of crashes: http://climate/reports/searchstacksignature.cfm?stacksig=MimeTypeElementImpl%3A%3AGetEnabledPlugin Andrei, are you looking into this? This needs to get on someone's radar...
Assignee | ||
Comment 7•23 years ago
|
||
Ahh! Andrei, it does crash with about:plugins, see bug 91569. Steps to reproduce: 1. Launch the browser. 2. Go help menu by following steps: a. Feedback center. b. About Plug-ins c. Feedback center again. The feedback url is: http://home.netscape.com/browsers/6/feedback/index61pr.html It also has to be that order. When I replaced "Feedback center" with another link, this did not happen.
Priority: -- → P1
On what platform do you experience this? Looks like this will be my first action item tomorrow...
Assignee | ||
Comment 9•23 years ago
|
||
Happens at least on Mac and Windows (W2k). I suspect all. My guess is that the mime-type array doesn't get cleared out all the way and it could be on the DOM side, but that's just a guess.
Summary: crash on reloading this page that does a plugins.refresh() → crash in MimeTypeElementImpl::GetEnabledPlugin after plugin.refresh()
Comment 10•23 years ago
|
||
Is this seen on the branch, or only the trunk?
Comment 11•23 years ago
|
||
Comment 12•23 years ago
|
||
I Just Created an attachment (id=42986) to illustrate my belief that the interaction between plugins.refresh() and the Universal Ad Call JS code is causing this bug. This attachment works fine in 6.0 and 6.01. I'm using the 2001070304 build of 6.1 b1 in Win98.
Comment 13•23 years ago
|
||
Talkback indicates that this is on both branch and trunk. http://climate/reports/searchstacksignature.cfm?stacksig=MimeTypeElementImpl%3A%3AGetEnabledPlugin
Assignee | ||
Comment 14•23 years ago
|
||
Looking closer at this with Brian Nesse, it looks like in nsGlocalWindow.cpp, NavigatorImpl caches the mime-type and plugin array. When a navigator.plugin.refresh() happens, the weak pointers to these plugins become stale and hence the crash when we revist a page that tried to access them. I think we need some way of having NavigatorImpl refresh itself w.r.t plugins and mimetypes when a navigator.plugins.refresh happens. Any ideas? I'm not familiar with which interfaces, if any in this area, are frozen.
Comment 15•23 years ago
|
||
This has shown up as a topcrasher for the N610 branch builds (and from Blake's last comment, this is happening on the MozillaTrunk as well). adding crash, topcrash keywords and [@ MimeTypeElementImpl::GetEnabledPlugin] to summary. Here is the latest info: MimeTypeElementImpl::GetEnabledPlugin 8 First BBID :32840758 Last BBID :33115944 Min Runtime :114 Max Runtime :85536 First Appearance Date : 2001-07-12 Last Appearance Date : 2001-07-19 First BuildID : 2001071106 Last BuildID : 2001071905 Stack Trace: MimeTypeElementImpl::GetEnabledPlugin() [nsMimeTypeArray.cpp line 217] CODE.10 XPTC_InvokeByIndex() [xptcinvoke_mac.cpp line 129] XPCWrappedNative::CallMethod() [xpcwrappednative.cpp line 1880] XPC_WN_GetterSetter() [xpcwrappednativejsops.cpp line 1284] js_Invoke() [jsinterp.c line 801] js_InternalInvoke() [jsinterp.c line 895] js_GetProperty() [jsobj.c line 2408] js_Interpret() [jsinterp.c line 2535] Source File : nsMimeTypeArray.cpp line : 217 (33112037) URL: popularcategories.com (33112037) Comments: Help -> Feedback (33109907) Comments: Mac crash on Help menu (32840758) URL: http://www.thewb.com/ (32840758) Comments: Went to the Bookmarks menu I also noticed that this crash is being reported under the 0x00000000 stack signature as well...so although there aren't "a lot" of these crashes, it is occurring under multiple stack signatures.
Assignee | ||
Comment 16•23 years ago
|
||
Okay, I think I have a fix for this. Hacking away, I made it public and called MimeTypeArrayImpl::GetMimeTypes(). That populated the mime-type array based on the plugin array. I did this in PluginArrayImpl::Refresh(), by way of doing an ugly static cast of mNavigator. I did not crash on the attached testcase or URL above. Looking at the interfaces involved here, I think I can clean this up neatly. I also need to take care of not leaking the old stuff as GetMimeTypes() doesn't account for it. However, I have some questions for an expert about the navigator.mimetype array: Specifically, would it be okay if I completely destroyed this array before any call to plugins.refresh() and then rebuilt it via a call to GetMimeTypes()? Does anything else but plugis use this array? Also, can I add GetMimeTypes() to an interface or should I use a NS_STATIC_CAST and just make it public? This is a top crash and good PDT+ material.
Assignee: av → peterlubczynski
Whiteboard: fix-in-hand
Target Milestone: --- → mozilla0.9.3
Comment 17•23 years ago
|
||
PDT+, we'll take the thoroughly reviewed and tested fix for this. Any extra eyes you can get on it would be appreciated.
Whiteboard: fix-in-hand → PDT+ fix-in-hand
Comment 18•23 years ago
|
||
Here's something that's a bit neater: 1) Pass in (and store) a NavigatorImpl* instead of an nsIDOMNavigator* to the PluginArrayImpl constructor. This will avoid the nasty (and potentially incorrect) casting. It's fine for these implementation classes to know about each other. 2) Add a public PluginRefresh() method to NavigatorImpl. This does not have to be part of any public interface. 3) Add a public Refresh() method to MimeTypeArrayImpl. This is the one that can clean up an existing mMimeTypeArray. It will be recreated lazily the next time it's needed. This method also does not have to be part of any public interface. 4) From NavigatorImpl::PluginRefresh() call MimeTypeArrayImpl::Refresh().
Assignee | ||
Comment 19•23 years ago
|
||
Assignee | ||
Comment 20•23 years ago
|
||
Here's a patch that should fix this top crash by completely cleaning out the mime type array and re-populating it after a navigator.plugin.refresh() happens. I'm seeking reviews (and some testing help).
Assignee | ||
Comment 21•23 years ago
|
||
Oh, I got my postfix and prefix operators mixed up last night. This: + while (--mMimeTypeCount) ...should be... + while (mMimeTypeCount--)
Review comments on attachment 43095 [details] [diff] [review] as modified by previous (10:53) comment: I'm hoping this will also fix bug 86642. :-) Since Vidur suggested this approach, I'll assume there was never any intent of plugging in a replacement for one of these interfaces without replacing the other. For MimeTypeArrayImpl::Refresh, why don't you do the loop releasing members of the array and freeing of the array itself the same way it's done in the destructor? (In fact, you could even refactor that loop in the destructor into an additional (inline) method.) At your new call site you would also have to set the array to null and the count to 0 after the loop since you're going to continue to use the object. There are two reasons I prefer this: * the |while (mMimeTypeCount--)| type loop is confusing, as demonstrated by your confusion (and mine :-) * you leak the array itself, and may end up leaving a dangling array of some sort in some cases that confuses other code (non-null array, but count of 0) To prevent someone else from making the same mistake in the future, could you add something like: NS_PRECONDITION(!mMimeTypeArray && mMimeTypeCount==0, "already initialized") to the very beginning of GetMimeTypes? With those comments addressed, looks good to me, r=dbaron.
Comment 23•23 years ago
|
||
Peter, I don't see mMimeTypeArray deleted before getting new one. Also, would be probably safer to zero it too. So, against your patch it could be something like this: nsresult MimeTypeArrayImpl::Refresh() { // clear out old array if (mMimeTypeArray) +{ while (mMimeTypeCount--) // DON'T FORGET THIS POSTFIX!!! NS_IF_RELEASE(mMimeTypeArray[mMimeTypeCount]); + + delete [] mMimeTypeArray; + mMimeTypeArray = nsnull; + } // fill it back up return GetMimeTypes(); } Other than that -- r=av On a different note, is this also going to fix (I don't remember the bug number) not refreshing mime type list while refreshing plugins?
Assignee | ||
Comment 24•23 years ago
|
||
Wow, this may take care of three birds with one stone (topcrash, topcrash, and mime refreshing)... Cleand up patch on it's way with a new factored Clear() method. dbaron, can I get a final sr=? Thanks!
Assignee | ||
Comment 25•23 years ago
|
||
r=dbaron on the new version, but you'll need to find a super-reviewer for an sr.
Assignee | ||
Updated•23 years ago
|
Whiteboard: PDT+ [SEEKING REVIEWS] → PDT+ [SEEKING SUPER REVIEW]
Comment 27•23 years ago
|
||
r=av on the last patch
Comment 28•23 years ago
|
||
Tabs, eww. sr=brendan@mozilla.org. /be
Assignee | ||
Comment 29•23 years ago
|
||
Patch checked into trunk and branch, marking FIXED.
Blocks: 86642
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Whiteboard: PDT+ [SEEKING SUPER REVIEW] → PDT+
Comment 30•23 years ago
|
||
looks ok on all 3 branch builds today 0723. marking verif. adding 'vtrunk ' keywd to verify on trunk later.
Status: RESOLVED → VERIFIED
Comment 33•23 years ago
|
||
*** Bug 90435 has been marked as a duplicate of this bug. ***
Updated•13 years ago
|
Crash Signature: [@ 0x00000000 - MimeTypeElementImpl::GetEnabledPlugin]
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•