The default bug view has changed. See this FAQ.

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

RESOLVED FIXED

Status

()

Core
DOM
--
major
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: Shailen, Assigned: Shailen)

Tracking

({verified1.9.0.19, verified1.9.1, verified1.9.2})

Trunk
PowerPC
AIX
verified1.9.0.19, verified1.9.1, verified1.9.2
Points:
---

Firefox Tracking Flags

(status1.9.2 .2-fixed, status1.9.1 .9-fixed)

Details

Attachments

(4 attachments, 8 obsolete attachments)

1.12 KB, patch
jst
: review+
Details | Diff | Splinter Review
877 bytes, patch
jst
: review+
Details | Diff | Splinter Review
1.30 KB, patch
Details | Diff | Splinter Review
1.07 KB, patch
Details | Diff | Splinter Review
(Assignee)

Description

8 years ago
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.
(Assignee)

Updated

8 years ago
OS: Other → AIX
(Assignee)

Comment 1

8 years ago
Created attachment 372792 [details] [diff] [review]
Root Cause :
Attachment #372792 - Flags: superreview?
Attachment #372792 - Flags: review?
(Assignee)

Comment 2

8 years ago
Created attachment 372795 [details] [diff] [review]
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.
Attachment #372792 - Attachment is obsolete: true
Attachment #372795 - Flags: superreview?(roc)
Attachment #372795 - Flags: review?(roc)
Attachment #372792 - Flags: superreview?
Attachment #372792 - Flags: review?
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?
(Assignee)

Comment 4

8 years ago
Created attachment 372838 [details] [diff] [review]
Patch V 2

Made the changes to the file nsJSEnvironment.cpp instead of nsGlobalWindow.cpp
Attachment #372795 - Attachment is obsolete: true
Attachment #372838 - Flags: superreview?(jst)
Attachment #372838 - Flags: review?(jst)
Attachment #372795 - Flags: superreview?(roc)
Attachment #372795 - Flags: review?(roc)
(Assignee)

Comment 5

8 years ago
Can someone please review the patch ?
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!
Attachment #372838 - Flags: superreview?(jst)
Attachment #372838 - Flags: superreview+
Attachment #372838 - Flags: review?(jst)
Attachment #372838 - Flags: review+
(Assignee)

Updated

8 years ago
Keywords: checkin-needed
Assignee: nobody → shailen.n.jain
Status: UNCONFIRMED → ASSIGNED
Component: General → DOM
Ever confirmed: true
Product: Firefox → Core
QA Contact: general → general
Target Milestone: --- → mozilla1.9.2a1
Version: unspecified → Trunk
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.
Please, also attach a hg patch, not a cvs one.
Keywords: checkin-needed
Whiteboard: [needs m-c landing]
Whiteboard: [needs m-c landing]
(Assignee)

Comment 10

7 years ago
Created attachment 418491 [details] [diff] [review]
Revised patch
Attachment #372838 - Attachment is obsolete: true
Attachment #418491 - Flags: superreview?(jst)
Attachment #418491 - Flags: review?(jst)
(Assignee)

Comment 11

7 years ago
The revised patch created using hg
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.
Blocks: 537588
(Assignee)

Updated

7 years ago
Attachment #418491 - Flags: approval1.9.2.1?
Attachment #418491 - Flags: approval1.9.1.8?
Attachment #418491 - Flags: approval1.9.2.1?
Attachment #418491 - Flags: approval1.9.1.8?
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.
(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.

Updated

7 years ago
Attachment #418491 - Flags: superreview?(jst)
Attachment #418491 - Flags: superreview+
Attachment #418491 - Flags: review?(jst)
Attachment #418491 - Flags: review+
(Assignee)

Updated

7 years ago
Keywords: checkin-needed
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.
Keywords: checkin-needed

Comment 16

7 years ago
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?
The suggested patch from #c16 looks nice, and is closer to the original code as it also handles argc == 0 case gracefully.
Hardware: Other → PowerPC
Target Milestone: mozilla1.9.2a1 → ---

Comment 18

7 years ago
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.
Created attachment 429240 [details] [diff] [review]
Revised to comments 16 and 18

Revised patch for trunk
Attachment #429240 - Flags: review?
Attachment #429240 - Flags: review? → review?(jst)
Created attachment 429241 [details] [diff] [review]
Same patch, context adjusted for branch 1.9.2

same as trunk patch, only context adjustment
Attachment #429241 - Flags: review?
Attachment #429241 - Flags: review? → review?(jst)
Created attachment 429242 [details] [diff] [review]
Same patch, moved file for branch 1.9.1

Branch 1.9.1 has source file moved to different directory
Attachment #429242 - Flags: review?(jst)
Created attachment 429248 [details] [diff] [review]
branch patch for 1.9.0

created with CVS
Attachment #429248 - Flags: review?(jst)
Created attachment 429250 [details] [diff] [review]
branch patch for 1.9.2, context adjusted only to trunk patch
Attachment #429241 - Attachment is obsolete: true
Attachment #429250 - Flags: review?(jst)
Attachment #429241 - Flags: review?(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

7 years ago
(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.
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.
-  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.
Created attachment 430731 [details] [diff] [review]
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.
Attachment #430731 - Flags: superreview+
Attachment #430731 - Flags: review+
Checked in, marking FIXED.

http://hg.mozilla.org/mozilla-central/rev/c05d0ac56307
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
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.
Attachment #429240 - Flags: review?(jst) → review-

Updated

7 years ago
Attachment #429242 - Flags: review?(jst) → review-

Updated

7 years ago
Attachment #429248 - Flags: review?(jst) → review-

Updated

7 years ago
Attachment #429250 - Flags: review?(jst) → review-
Comment on attachment 430731 [details] [diff] [review]
Shailen's patch with a fix.

Fine to have this fixed :-)
Requesting approval for branches.
Attachment #430731 - Flags: approval1.9.2.2?
Attachment #430731 - Flags: approval1.9.1.9?
Attachment #430731 - Flags: approval1.9.0.19?
Attachment #429250 - Attachment is obsolete: true
Attachment #429248 - Attachment is obsolete: true
Attachment #429242 - Attachment is obsolete: true
Attachment #429240 - Attachment is obsolete: true
Created attachment 430815 [details] [diff] [review]
fixed patch adjusted for branch 1.9.0

unit moved to different src dir and context adjusted. for CVS
Created attachment 430816 [details] [diff] [review]
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
Attachment #430731 - Flags: approval1.9.1.9?
Attachment #430731 - Flags: approval1.9.0.19?
Attachment #430815 - Flags: approval1.9.0.19?
Attachment #430816 - Flags: approval1.9.1.9?
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.
Attachment #430816 - Flags: approval1.9.1.9? → approval1.9.1.9+
Attachment #430815 - Flags: approval1.9.0.19? → approval1.9.0.19+
Attachment #430731 - Flags: approval1.9.2.2? → approval1.9.2.2+
Keywords: checkin-needed
Whiteboard: [checkin to cvs] [checkin to mozilla-1.9.1] [checkin to mozilla-1.9.2]
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
status1.9.1: --- → .9-fixed
status1.9.2: --- → .2-fixed
Keywords: fixed1.9.0.19
Keywords: checkin-needed
Whiteboard: [checkin to cvs] [checkin to mozilla-1.9.1] [checkin to mozilla-1.9.2]
Keywords: fixed1.9.0.19 → verified1.9.0.19
Keywords: verified1.9.1, verified1.9.2
You need to log in before you can comment on or make changes to this bug.