488423 - JavaScript error: , line 0: uncaught exception: [Exception... Component returned failure code: 0x8007000e (NS_ERROR_OUT_OF_MEMORY) [nsIDOMJSWindow.openDialog]

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Shailen]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.8) Gecko/2009032609 Firefox/3.0.8
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.8) Gecko/2009032609 Firefox/3.0.8

Launch the browser Firefox on AIX, and click on Help->About Mozilla Firefox, it gives the following error

'Uncaugth exception : "Component returned failur code : 0x8007000e  (NS_ERROR_OUT_OF_MEMORY)
(nsIDOMJSWINDOW.openDialog)" nsresult: 0x8007000e  (NS_ERROR_OUT_OF_MEMORY) location "JS frame ::
chrome://browser/content/utilityOverlay.js :: openAboutDialog :: line 340" data: no]

Reproducible: Always

Steps to Reproduce:
1. Build Mozilla Firefox Trunk build on AIX with Gnome RPMS of 64 bit
version.
2. Launch the browser 
3. Click on Help->About Mozilla Firefox
4. The Dialog box does not comeup.

Actual Results:  
1. Click on Tools->Java Console
2. The error is displayed as stated above.

Expected Results:  
Should display the dialog box without giving any error.

Comment 1

[Shailen]

Attachment: Root Cause :

Comment 2

[Shailen]

Attachment: Patch V 1

Root Cause : The second parameter(argc-argOffset) of the function NS_CreateJSArgv evaluates to 0 (zero) which inturn results in calling the function PR_CALLOC with 0 (zero) as the parameter. 

As per the path, the above condition does not occur.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I don't think this patch is correct. I think the bug is here:
http://mxr.mozilla.org/mozilla-central/source/dom/base/nsJSEnvironment.cpp#4000
It should return NS_OK if argc is 0. I'm not the right reviewer for this code though, try jst?

Comment 4

[Shailen]

Attachment: Patch V 2

Made the changes to the file nsJSEnvironment.cpp instead of nsGlobalWindow.cpp

Comment 5

[Shailen]

Can someone please review the patch ?

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

jst?

Comment 7

[Johnny Stenback (:jst)]

Comment on attachment 372838 [details] [diff] [review]
Patch V 2

-  mArgv = (jsval *) PR_CALLOC(argc * sizeof(jsval));
-  if (!mArgv) {
-    *prv = NS_ERROR_OUT_OF_MEMORY;
-    return;
+  if (argc > 0) {
+    mArgv = (jsval *) PR_CALLOC(argc * sizeof(jsval));
+    if (!mArgv) {
+      *prv = NS_ERROR_OUT_OF_MEMORY;
+      return;
+    }
   }
-

Looks good, but I'd leave the empty space in place here.

r+sr=jst, thanks for the patch!

Comment 8

[Phil Ringnalda (:philor)]

I pushed this to the try server, and both Linux and Windows unit tests claimed that they crashed in mochitest-browser-chrome. Maybe coincidence, but I wouldn't risk pushing it without getting a clean try server run first.

Comment 9

[Serge Gautherie (:sgautherie)]

Please, also attach a hg patch, not a cvs one.

Comment 10

[Shailen]

Attachment: Revised patch

Comment 11

[Shailen]

The revised patch created using hg

Comment 12

[Uli Link (:ul-mcamafia)]

Shailen: can you request approval for 1.9.1 branch.

I have tested this patch with Firefox 3.0.17, 3.5.7, Seamonkey 2.0.1 and Thunderbird 3.0. They all need this patch.
Seamonkey cannot start the MailNews Client without this patch.

Comment 13

[Daniel Veditz [:dveditz]]

Comment on attachment 418491 [details] [diff] [review]
Revised patch

Please don't ask for branch approval until we've got a fully-reviewed and tested patch.

Comment 14

[Uli Link (:ul-mcamafia)]

(In reply to comment #13)
> (From update of attachment 418491 [details] [diff] [review])
> Please don't ask for branch approval until we've got a fully-reviewed and
> tested patch.

https://bugzilla.mozilla.org/attachment.cgi?id=372838 had both r + sr.
As the only difference the newer patch was created using hg.
Since May no further reaction to https://bugzilla.mozilla.org/show_bug.cgi?id=488423#c8 appeared. Maybe shailen better not checked the obsolete button for the new patch, but then there maybe a confusion which patch to use when it comes to checkin.

Comment 15

[Phil Ringnalda (:philor)]

http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1264692306.1264699745.22461.gz&fulltext=1
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1264654267.1264663363.6641.gz&fulltext=1
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1264654266.1264662599.30902.gz&fulltext=1
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1264692306.1264702806.28026.gz&fulltext=1

Some of that is just the usual random test failures, but consistently failing every single mochitest run on both Windows and Linux in two tryserver runs is beyond just random.

Comment 16

[The Written Word]

We used this patch to work around the same issue (seamonkey-2.0.3 on AIX, trying to open Mail & News or chatzilla from the menu does nothing, and in a debug build prints the NS_ERROR_OUT_OF_MEMORY error).

Index: mozilla/dom/src/base/nsJSEnvironment.cpp
===================================================================
--- mozilla/dom/src/base/nsJSEnvironment.cpp.orig       2010-02-06 01:03:55.000000000 +0000
+++ mozilla/dom/src/base/nsJSEnvironment.cpp    2010-02-26 20:34:57.625303594 +0000
@@ -4104,7 +4104,7 @@
 {
   // copy the array - we don't know its lifetime, and ours is tied to xpcom
   // refcounting.  Alloc zero'd array so cleanup etc is safe.
-  mArgv = (jsval *) PR_CALLOC(argc * sizeof(jsval));
+  mArgv = (jsval *) PR_CALLOC(argc ? argc * sizeof(jsval): sizeof(jsval));
   if (!mArgv) {
     *prv = NS_ERROR_OUT_OF_MEMORY;
     return;


PR_CALLOC(0) seems to be expected to give a non-zero result, however, a quick test on AIX 6.1 using calloc(0,4) shows that it returns 0, calloc returning 0 is allowed - http://www.opengroup.org/onlinepubs/9699919799/functions/calloc.html

Should the implementation of PR_Calloc instead be fixed to never return NULL?

Comment 17

[Uli Link (:ul-mcamafia)]

The suggested patch from #c16 looks nice, and is closer to the original code as it also handles argc == 0 case gracefully.

Comment 18

[Wan-Teh Chang]

malloc/calloc/realloc all have this issue when their 'size' argument is 0.
The return value in that case is implementation-dependent.

It's best to avoid calling these functions with a 'size' of 0, as the
patch does.

Comment 19

[Uli Link (:ul-mcamafia)]

Attachment: Revised to comments 16 and 18

Revised patch for trunk

Comment 20

[Uli Link (:ul-mcamafia)]

Attachment: Same patch, context adjusted for branch 1.9.2

same as trunk patch, only context adjustment

Comment 21

[Uli Link (:ul-mcamafia)]

Attachment: Same patch, moved file for branch 1.9.1

Branch 1.9.1 has source file moved to different directory

Comment 22

[Uli Link (:ul-mcamafia)]

Attachment: branch patch for 1.9.0

created with CVS

Comment 23

[Uli Link (:ul-mcamafia)]

Attachment: branch patch for 1.9.2, context adjusted only to trunk patch

Comment 24

[Johnny Stenback (:jst)]

I just pushed the initial patch to the try server since I'd like to understand
why it's not good enough. Allocating a sizeof(jsval) that'll never be used
seems unnecessary to me, so I'd prefer to understand why the original patch
didn't stick.

Comment 25

[The Written Word]

(In reply to comment #24)
> I just pushed the initial patch to the try server since I'd like to understand
> why it's not good enough.

Ok. We figured a NULL mArgv was not right when we saw:

nsresult
nsJSArgArray::GetArgs(PRUint32 *argc, void **argv)
{
  if (!mArgv) {
    NS_WARNING("nsJSArgArray has no argv!");
    return NS_ERROR_UNEXPECTED;
  }

in the same file.

Comment 26

[Phil Ringnalda (:philor)]

Conveniently enough, I pushed it about five minutes after you, and we both agree: failed all four flavors of mochitest, on both Windows and Linux.

Comment 27

[Uli Link (:ul-mcamafia)]

-  mArgv = (jsval *) PR_CALLOC(argc * sizeof(jsval));
-  if (!mArgv) {
-    *prv = NS_ERROR_OUT_OF_MEMORY;
-    return;
+  if (argc = 0) {
+     *prv = NS_OK;
+     return;
+  }
+  if (argc > 0) {
+    mArgv = (jsval *) PR_CALLOC(argc * sizeof(jsval));
+    if (!mArgv) {
+      *prv = NS_ERROR_OUT_OF_MEMORY;
+      return;
+    }
   }

So it will return for the argc = 0 case and also not call calloc with 0 which will return a NULL pointer on AIX.
Only this case is different for the original patch compared to the repository.
Slightly different to the TWW proposal from comment #16

Cross checked with aCC on HP-UX: same behaviour as Linux or Windows.
So if this also fails the try server, I suggest a #ifdef __IBMCPP__'ed workaround as a last resort.

Comment 28

[Johnny Stenback (:jst)]

Attachment: Shailen's patch with a fix.

This is Shailen's fix with the additional change to not set mArgv to argv so that it remains null when calling this code with 0 passed as argc. W/o that additional change any such calls end up freeing the passed in argv when the nsJSArgArray gets destroyed.

Comment 29

[Johnny Stenback (:jst)]

Checked in, marking FIXED.

http://hg.mozilla.org/mozilla-central/rev/c05d0ac56307

Comment 30

[Johnny Stenback (:jst)]

Comment on attachment 429240 [details] [diff] [review]
Revised to comments 16 and 18

If we need to do anything for this for the branches, we should do the same as what landed on the trunk, not this approach.

Comment 31

[Uli Link (:ul-mcamafia)]

Comment on attachment 430731 [details] [diff] [review]
Shailen's patch with a fix.

Fine to have this fixed :-)
Requesting approval for branches.

Comment 32

[Uli Link (:ul-mcamafia)]

Attachment: fixed patch adjusted for branch 1.9.0

unit moved to different src dir and context adjusted. for CVS

Comment 33

[Uli Link (:ul-mcamafia)]

Attachment: fixed patch adjusted for branch 1.9.1

applied trunk patch in moved src dir with patch -p3 and recreated with hg for context adjustmend. 1.9.1 branch

Comment 34

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 430816 [details] [diff] [review]
fixed patch adjusted for branch 1.9.1

a=beltzner for branches, please get someone to check these in ASAP.

Comment 35

[Johnny Stenback (:jst)]

Fixed on branches and in CVS.

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/44ff90620a84
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/76d7fe0b8680