Closed Bug 354595 Opened 18 years ago Closed 17 years ago

Crash setting flashEmbed[0] in Flash Player code called from nsJSObjWrapper::GetNewOrUsed


(Core Graveyard :: Plug-ins, defect)

Not set


(Not tracked)



(Reporter: jruderman, Unassigned)


(Keywords: crash, testcase, verified1.8.1.8)


(3 files)

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
Summary: Crash setting flashEmbed[0] → Crash setting flashEmbed[0] in Flash Player code called from nsJSObjWrapper::GetNewOrUsed
Attached file testcase
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.
Attached patch Possible fix.Splinter Review
Attachment #261878 - Flags: superreview?(jonas)
Attachment #261878 - Flags: review?(jonas)
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.
Attachment #261878 - Flags: superreview?(jonas)
Attachment #261878 - Flags: superreview+
Attachment #261878 - Flags: review?(jonas)
Attachment #261878 - Flags: review+
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 on attachment 261878 [details] [diff] [review]
Possible fix.

approved for, a=dveditz
Attachment #261878 - Flags: approval1.8.1.7+
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.
Marking bug fixed as there hasn't been any sign of this being a problem since this patch landed on the trunk.
Closed: 17 years ago
Resolution: --- → FIXED
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).
Fixed on branch.
Keywords: fixed1.8.1.7
Attachment #275675 - Attachment is patch: true
Attachment #275675 - Attachment mime type: application/octet-stream → text/plain
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
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
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.
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.
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.