Last Comment Bug 311388 - crash [@ nsPluginHostImpl::IsPluginEnabledForExtension]
: crash [@ nsPluginHostImpl::IsPluginEnabledForExtension]
Status: RESOLVED FIXED
: crash, fixed1.8, helpwanted, topcrash+
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: 1.8 Branch
: x86 Windows XP
: -- critical (vote)
: ---
Assigned To: Johnny Stenback (:jst, jst@mozilla.com)
:
Mentors:
http://www.msn.com
: 313905 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-10-06 12:27 PDT by Tracy Walker [:tracy]
Modified: 2011-08-05 21:27 PDT (History)
14 users (show)
jaymoz: blocking1.7.14?
mscott: blocking1.8b5-
asa: blocking1.8rc1+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
all incidents from fastfind (html/zipped) (288.37 KB, application/octet-stream)
2005-10-06 14:33 PDT, Peter van der Woude [:Peter6]
no flags Details
about:plugins (26.30 KB, text/html)
2005-10-10 07:15 PDT, Tracy Walker [:tracy]
no flags Details
zip of quicktime plugin files (140.80 KB, application/x-zip-compressed)
2005-10-18 22:59 PDT, Henrik Gemal
no flags Details
Null-check plugins->mExtensionsArray since it can be null. (2.14 KB, patch)
2005-10-19 15:02 PDT, Johnny Stenback (:jst, jst@mozilla.com)
no flags Details | Diff | Review
diff -w of the above. (1.88 KB, patch)
2005-10-19 15:03 PDT, Johnny Stenback (:jst, jst@mozilla.com)
mrbkap: review+
darin.moz: superreview+
asa: approval1.8rc1+
Details | Diff | Review

Description Tracy Walker [:tracy] 2005-10-06 12:27:29 PDT
I can't realiably reproduce this crash but have crashed at least once with build
from 1005 and 1006 at cnn.com

My crash reprot from this morning
TB10291276K
A query for nsPluginHostImpl::IsPluginEnabledForExtension against 1.5 beta
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=1&searchby=stacksig&match=contains&searchfor=+nsPluginHostImpl%3A%3AIsPluginEnabledForExtension&vendor=All&product=Firefox15&platform=All&buildid=&sdate=&stime=&edate=&etime=&sortby=bbid

other topsites crash reports mention:
my.yahoo.com
msn
ebay
geocities
weather.com
Comment 1 Tracy Walker [:tracy] 2005-10-06 12:36:31 PDT
There has been 1000 reports of this crash signature since 9/16.

Nominationg for beta2 and rc1 to get on the radar and let drivers decide which
bucket it belongs
Comment 2 Peter van der Woude [:Peter6] 2005-10-06 13:13:14 PDT
almost all (997 out of 1001) reports were from MozillaOrgFirefox15Win322005090806

I'm tempted to think your crash was coincidence (branch nighlyhourly testers are
notorious TB submitters , this one should have been extremely visable)

It started on 20050916, which is odd, it should have started the day the build
was released.

Maybe this was caused by M$ (windows update) or maybe an incredibly popular
extension or plugin that was updated.

I checked the few pages that were mentioned in the bugs and they have different
media (some flash,some wmp, some QT)
Comment 3 Henrik Gemal 2005-10-06 13:43:17 PDT
I've seen this on trunk nightly builds as well:
TB10299389W
Comment 4 Peter van der Woude [:Peter6] 2005-10-06 14:33:02 PDT
Created attachment 198732 [details]
all incidents from fastfind (html/zipped)

for all FF versions
total # of incidents = 7621 crashes
lowest FF version affected = MozillaOrgFirefox10Win322005071605
startdate = 20050916
Comment 5 Scott MacGregor 2005-10-06 16:46:36 PDT
I couldn't find any recent branch talkback crashes with this stack trace in
talkback. They were all from 9/16 as Peter said. But maybe Talkback is still
behind. Can you do some digging Jay?
Comment 6 Jay Patel [:jay] 2005-10-06 17:09:40 PDT
I don't see too many of these crashes in 1.5 branch data, but there are enough
for Beta 1 and other recent releases to take a look at this crash for RC1.

I have not been able to reproduce this, but if we can find a way to consistently
see this crash and/or develop a testcase, this should be fixed.
Comment 7 Christian :Biesinger (don't email me, ping me on IRC) 2005-10-07 06:17:10 PDT
this may depend on some special plugin being installed, does talkback list the
loaded DLLs?
Comment 8 Jay Patel [:jay] 2005-10-07 11:42:11 PDT
The DLL info is difficult to pull out of the database, but I can use the
official Talkback report system to see what's there for some of these crashes. 
I will do that next week when I get back from vacation.
Comment 9 Henrik Gemal 2005-10-08 01:01:24 PDT
I haev these plugins installed when I crashed:
- QuickTime Plug-in 6.5.1
- Shockwave Flash 8.0 r22
- RealPlayer(tm) G2 LiveConnect-Enabled Plug-In (32-bit)
- Macromedia Shockwave for Director Netscape plug-in, version 10.1
- Java(TM) 2 Platform Standard Edition 5.0 Update 4
- Windows Media Player Plug-in Dynamic Link Library
Comment 10 Tracy Walker [:tracy] 2005-10-10 07:13:53 PDT
I can reliably reproduce this crash in the following manner.

-Delete the Firefox profile folder
-Launch Fx (don't import anything)
-browse to www.msn.com from the location bar

crash in nsPluginHostImpl::IsPluginEnabledForExtension
TB ID: 10468942

Subsequent attempts to visit msn.com result in no crash.
Comment 11 Tracy Walker [:tracy] 2005-10-10 07:15:12 PDT
Created attachment 199065 [details]
about:plugins
Comment 12 Asa Dotzler [:asa] 2005-10-10 16:36:19 PDT
Johnny, we've got good steps to reproduce and this is the early favorite in the
topcrash data. Can you take a look?
Comment 13 Asa Dotzler [:asa] 2005-10-11 14:51:32 PDT
->jst in hope that he can help us out.
Comment 14 Asa Dotzler [:asa] 2005-10-18 18:33:01 PDT
I can't reproduce with the same set of plugins installed but I noticed that
tracy and henrik both have quicktime 6.5.1 where I have the newer 7 version. Can
one of you attach the older version to this bug so I can test that. 
Comment 15 Henrik Gemal 2005-10-18 22:59:31 PDT
Created attachment 200053 [details]
zip of quicktime plugin files
Comment 16 Henrik Gemal 2005-10-18 23:12:30 PDT
I use the K-Lite Mega Codec Pack which includes the QT version 6.5.1
http://www.codecguide.com/download_mega.htm
Comment 17 Johnny Stenback (:jst, jst@mozilla.com) 2005-10-19 12:37:28 PDT
Tracy, I still haven't been able to reproduce this crash. I installed the right
quicktime plugin version n' all, and still no go. So it might be some other
plugin that's causing this. From looking at the code involved and your
about:plugins that you attached, I'd suspect it's this plugin:

    File name: npYState.dll

MIME Type 	Description 	Suffixes 	Enabled
application/x-vnd.yahoo.applicationState 			Yes

The key there is that it doesn't advertise an file suffixes and I think that
might be the problem, but only when the plugins are initially registerd.

Tracy, do you think you could attach that plugin to this bug, or email it
directly to me? I've got an idea on how to fix this, assuming my theory is
right, but I'd like to make sure if possible...
Comment 18 Henrik Gemal 2005-10-19 12:52:31 PDT
I dont have that plugin install and I can still crash.
All the plugins in about:plugins is "enabled" and it's only the java ones that
lack text in "Suffixes"
Comment 19 Jay Patel [:jay] 2005-10-19 13:12:16 PDT
I have not been able to reproduce this at all.  I've tried Tracy's steps and a
number of combinations of plugins and extensions, and still no crash...

Tracy:  What extensions do you have installed?  I'm guessing none, since you are
running from a fresh profile directory with nothing imported.  

If anyone that's seeing this crash can post all plugins + extensions that will
be great.
Comment 20 Tracy Walker [:tracy] 2005-10-19 13:29:28 PDT
I haven't been able to reproduce this crash the past couple of days at msn.com.
 I went back and tried with builds I had crashed before. No crash. maybe msn.com
changed content?  I've been trying to get to the talkback reports without
success today.  I'd like to find another site that this is reorted to be
crashing on.

jst: do just want the npYState.dll? (this is part of the Yahoo! Messenger
installation)
Comment 21 Johnny Stenback (:jst, jst@mozilla.com) 2005-10-19 15:02:45 PDT
Created attachment 200145 [details] [diff] [review]
Null-check plugins->mExtensionsArray since it can be null.

This should fix this bug. I have yet to see this happen, but I did get the
Yahoo plugin dll from Tracy and with that we end up with a plugin info object
with a null mExtensionsArray member on first startup after installing the
plugin, the reason being that the plugin doesn't support any file extensions at
all. On subsequent startups we'll get this info from the pluginreg.dat file
that we keep, and in that case we'll get a non-null array of extensions (all
empty), so we don't crash in that case, but on first startup when we get this
info from the plugin we do.

So w/o actually seeing this crash, this is the best I've got. Oh, and the first
part of this patch is unrelated, but IMO correct. PL_strncasecmp() will walk
over n characters, or until it reaches a null terminator in the *first*
argument, but it'll happily walk past null terminators in the second argument,
so the order of the arguments is incorrect in that call.

diff -w version to follow.
Comment 22 Johnny Stenback (:jst, jst@mozilla.com) 2005-10-19 15:03:21 PDT
Created attachment 200146 [details] [diff] [review]
diff -w of the above.
Comment 23 Blake Kaplan (:mrbkap) (please use needinfo!) 2005-10-19 15:09:11 PDT
Comment on attachment 200146 [details] [diff] [review]
diff -w of the above.

r=mrbkap
Comment 24 Darin Fisher 2005-10-19 17:05:44 PDT
Comment on attachment 200146 [details] [diff] [review]
diff -w of the above.

>Index: modules/plugin/base/src/nsPluginHostImpl.cpp

>   const char *pExt = aExtensionList;
>   const char *pComma = strchr(pExt, ',');
> 
>   if(pComma == nsnull)
>     return PL_strcasecmp(pExt, aExtension);
> 
>   while(pComma != nsnull)
>   {
>     int length = pComma - pExt;
>-    if(0 == PL_strncasecmp(pExt, aExtension, length))
>+    if(0 == PL_strncasecmp(aExtension, pExt, length))
>       return 0;
> 
>     pComma++;
>     pExt = pComma;
>     pComma = strchr(pExt, ',');
>   }

I'm probably missing something, but I don't see how strlen(aExtension)
could ever be less than length.


The other part of this change looks nice and consistent with the rest
of the code that null checks mExtensionsArray before dereferencing it.

sr=darin
Comment 25 Darin Fisher 2005-10-19 17:18:14 PDT
> >   const char *pExt = aExtensionList;

nevermind, i kept reading this as:

      const char *pExt = aExtension;

which explains my crazy comment about strlen(aExtension).
Comment 26 Johnny Stenback (:jst, jst@mozilla.com) 2005-10-19 17:30:13 PDT
Fix checked in on the trunk.
Comment 27 Johnny Stenback (:jst, jst@mozilla.com) 2005-10-20 16:14:09 PDT
Fixed on the 1.8 branch now too.
Comment 28 Adam Guthrie 2005-10-31 06:59:17 PST
*** Bug 313905 has been marked as a duplicate of this bug. ***
Comment 29 Jay Patel [:jay] 2006-04-24 22:57:14 PDT
This might be a major topcrasher for Mozilla Suite 1.7.13.  It is #1 on the list so far (http://talkback-public.mozilla.org/reports/mozilla/M1713/M1713-topcrashers.html) and although the stacks vary in http://talkback-public.mozilla.org/reports/mozilla/M1713/smart-analysis.all, the stack signature is the same.

I wonder if this null check will fix the crash on the 1.7 branch. 

Johnny?

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