As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact
Last Comment Bug 354595 - Crash setting flashEmbed[0] in Flash Player code called from nsJSObjWrapper::GetNewOrUsed
: Crash setting flashEmbed[0] in Flash Player code called from nsJSObjWrapper::...
: crash, testcase, verified1.8.1.8
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: PowerPC Mac OS X
: -- critical (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
: Benjamin Smedberg [:bsmedberg]
Depends on:
Blocks: 326633
  Show dependency treegraph
Reported: 2006-09-27 17:29 PDT by Jesse Ruderman
Modified: 2007-10-22 11:29 PDT (History)
9 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

testcase (480 bytes, text/html)
2006-09-27 17:30 PDT, Jesse Ruderman
no flags Details
Possible fix. (2.75 KB, patch)
2007-04-17 16:05 PDT, Johnny Stenback (:jst,
jonas: review+
jonas: superreview+
dveditz: approval1.8.1.8+
Details | Diff | Splinter Review
Branch version, for the record (same change as above w/o some style cleanup). (2.02 KB, patch)
2007-08-07 15:07 PDT, Johnny Stenback (:jst,
no flags Details | Diff | Splinter Review

Description User image Jesse Ruderman 2006-09-27 17:29:16 PDT
This crashes if "emb" is a Flash movie:

  var emb = document.getElementById("emb")
  emb[0] = 3;

Stack trace (note that the symbols for Flash Player are bogus):

0   ...romedia.Flash Player.plugin 	0x06b63b38 Flash_EnforceLocalSecurity + 374272
1   ...romedia.Flash Player.plugin 	0x06b638a4 Flash_EnforceLocalSecurity + 373612
2   ...romedia.Flash Player.plugin 	0x06cce754 native_ShockwaveFlash_TCallFrame + 452328
3   ...romedia.Flash Player.plugin 	0x06cce6f4 native_ShockwaveFlash_TCallFrame + 452232
4   org.mozilla.firefox            	0x007f6e50 nsJSObjWrapper::GetNewOrUsed(_NPP*, JSContext*, JSObject*) + 2956
5   libmozjs.dylib                 	0x2304c2e0 js_LookupPropertyWithFlags + 684
6   libmozjs.dylib                 	0x23030b2c js_CheckRedeclaration + 84
7   libmozjs.dylib                 	0x23048ea8 js_HasOwnPropertyHelper + 1156
8   libmozjs.dylib                 	0x2302ff28 js_Invoke + 1828
9   libmozjs.dylib                 	0x2303a4d0 js_Interpret + 37512
Comment 1 User image Jesse Ruderman 2006-09-27 17:30:06 PDT
Created attachment 240387 [details]
Comment 2 User image Raymond Walsh 2007-04-17 12:17:05 PDT
It appears possible for PL_DHashTableOperate to return null (when the table is on the verge of overflowing and the object exists), but this condition is not trapped.

dhcp145:~/cvsrepos/mozilla/modules/plugin/base/src apple$ cvs diff -u8p ./nsJSNPRuntime.cpp 
Index: nsJSNPRuntime.cpp
RCS file: /cvsroot/mozilla/modules/plugin/base/src/nsJSNPRuntime.cpp,v
retrieving revision 1.23
diff -u -8 -p -r1.23 nsJSNPRuntime.cpp
--- nsJSNPRuntime.cpp   27 Mar 2007 15:33:41 -0000      1.23
+++ nsJSNPRuntime.cpp   17 Apr 2007 18:39:22 -0000
@@ -948,16 +948,19 @@ nsJSObjWrapper::GetNewOrUsed(NPP npp, JS
   nsJSObjWrapperKey key(obj, npp);
   JSObjWrapperHashEntry *entry =
     NS_STATIC_CAST(JSObjWrapperHashEntry *,
                    PL_DHashTableOperate(&sJSObjWrappers, &key, PL_DHASH_ADD));
+  if (!entry) {
+      return nsnull;
+  }
   if (PL_DHASH_ENTRY_IS_BUSY(entry) && entry->mJSObjWrapper) {
     // Found a live nsJSObjWrapper, return it.
     return _retainobject(entry->mJSObjWrapper);
   // No existing nsJSObjWrapper, create one.
Comment 3 User image Johnny Stenback (:jst, 2007-04-17 16:05:11 PDT
Created attachment 261878 [details] [diff] [review]
Possible fix.
Comment 4 User image Johnny Stenback (:jst, 2007-04-17 16:06:18 PDT
And that fix is of course based on Raymonds findings. I'm not sure yet whether it actually fixes this bug, but it's the right thing to do either way.
Comment 5 User image Johnny Stenback (:jst, 2007-04-17 16:11:14 PDT
Fix checked in on trunk. Leaving bug open as I don't know if this actually fixes this bug. If it appears to, feel free to mark this bug fixed.
Comment 6 User image Daniel Veditz [:dveditz] 2007-08-07 14:55:12 PDT
Comment on attachment 261878 [details] [diff] [review]
Possible fix.

approved for, a=dveditz
Comment 7 User image Johnny Stenback (:jst, 2007-08-07 14:55:18 PDT
Comment on attachment 261878 [details] [diff] [review]
Possible fix.

Requesting approval to land this on branches, I've gotten off-line reports of people seeing this crash on the branch, but not on the trunk since this landed there.
Comment 8 User image Johnny Stenback (:jst, 2007-08-07 14:56:07 PDT
Marking bug fixed as there hasn't been any sign of this being a problem since this patch landed on the trunk.
Comment 9 User image Johnny Stenback (:jst, 2007-08-07 15:07:57 PDT
Created attachment 275675 [details] [diff] [review]
Branch version, for the record (same change as above w/o some style cleanup).

This is the same as the trunk version with the exception of the first hunk that only changes code formatting (and doesn't apply to the branch).
Comment 10 User image Johnny Stenback (:jst, 2007-08-07 15:10:26 PDT
Fixed on branch.
Comment 11 User image Carsten Book [:Tomcat] 2007-09-13 07:35:29 PDT
verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv: Gecko/20070913 BonEcho/ ID:2007091303 

no crash on testcase - adding verified keyword
Comment 12 User image Daniel Veditz [:dveditz] 2007-10-22 10:49:09 PDT
I don't know if this is the cause, but Mac Flash crashes have skyrocketed in compared to  Even though total crashes in a 10 day window favor the old version 45K to 27K, crashes in Flash on a Mac have gone from 223 to 844.

FF             FF
223 Flash crashes      844 Flash crashes
13.5% of Mac crashes   60% of Mac crashes
0.5% of all crashes    3% of all crashes
Comment 13 User image Martijn Wargers [:mwargers] 2007-10-22 11:00:52 PDT
Yeah, I noticed quite a few Mac people complaining about crashing after the update in the Mozilla feedback newsgroups.
Carsten mentioned that it could be related to bug 400554.
I don't have a Mac, so I can't really help with testing.
Comment 14 User image Daniel Veditz [:dveditz] 2007-10-22 11:29:39 PDT
I filed bug 400714 to cover comment 12 -- this fix may or may not have been the cause so it should not be tracked here.

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