37837 - Crash while executing loadResources.

Status: VERIFIED FIXED

(Core Graveyard :: Installer: XPInstall Engine, defect, P3)

Description

[David Epstein]

all platforms. builds 2000-05-01-11-M16.
1. Go to http://jimbob/trigger3.html
2. Select a_loadresources from acceptance test case menu.
3. Trigger. OK.
4. Wait a few seconds (especially on Linux).
Result: Crash. no entry in logfile.

Comment 1

[David Epstein]

TalkBack log:

 Call Stack:    (Signature = nsInstall::LoadResources 8892f11c) 
     
   nsInstall::LoadResources                                       
[d:\builds\seamonkey\mozilla\xpinstall\src\nsInstall.cpp, line 1243]     
   InstallLoadResources                                       
[d:\builds\seamonkey\mozilla\xpinstall\src\nsJSInstall.cpp, line 1325]     
   js_Invoke                                       
[d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 687]     
   js_Interpret                                       
[d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 2484]     
   js_Execute                                       
[d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 859]     
   JS_EvaluateUCScriptForPrincipals                                       
[d:\builds\seamonkey\mozilla\js\src\jsapi.c, line 2749]     
   JS_EvaluateUCScript                                       
[d:\builds\seamonkey\mozilla\js\src\jsapi.c, line 2730]     
   JS_EvaluateScript                                       
[d:\builds\seamonkey\mozilla\js\src\jsapi.c, line 2697]     
   RunInstallOnThread                                       
[d:\builds\seamonkey\mozilla\xpinstall\src\nsSoftwareUpdateRun.cpp, line 432]     
   _PR_NativeRunThread 
                                      [pruthr.c, line 421]     
   MSVCRT.dll + 0x3820 (0x78003820)                                             
   KERNEL32.dll + 0x4ee8 (0x77f04ee8)                                        

Comment 2

[David Epstein]

changed QA contact to depstein. Added "crash" to keywords.

Comment 3

[Cathleen]

regression, must fix for b2

Comment 4

[Samir Gehani]

jbetak,
Recently you modified the guts of LoadResources in XPInstall.  can you 
investigate this regression, please?  Let me know if you need help getting set 
up.  Thanks.

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&fi
le=nsInstall.cpp&root=/cvsroot&subdir=mozilla/xpinstall/src&command=DIFF_FRAMESE
T&rev1=1.129&rev2=1.130

Comment 5

[Frank Tang]

jbetak- read http://www.mozilla.org/scriptable/faq.html#i8

According to this document, the implementation of nsIPropertyElement GetKey and 
GetValue should call nsAllocator to allocate memory and return. the caller 
shoudl call nsAllocator::Free to free the memory.

but in jbetak's recent change, The GetKey and GetValue implementation which call 
the ToNewUnicode() in nsString to allocate memory (see 
http://lxr.mozilla.org/seamonkey/source/xpcom/ds/nsPersistentProperties.cpp#417) 
, which indirectly call nsAllocator. However, he change the caller call delete[] 
pKey (and delete[] pValue) instead of 
nsAllocator::Free(pKey)/nsAllocator::Free(pValue). Which may cause this crash. 
(I am not 100% sure.)
 
Installer folks, can you try this in local build and see does it solve the crash 
?

Comment 6

[Samir Gehani]

Thanks for your input Frank.

Comment 7

[[e-mail] (away - not reading bugmail)]

Folks,

I have to respectfully disagree - try to install a build that was "baked" 10 
hours before my checkin and you will see the same crash (or so I 
hope) ...

ftp://ftp.mozilla.org/pub/mozilla/nightly/2000-05-01-11-M16/

While I agree with Frank that I could/should/would have used nsAllocator::Free 
instead of delete[], I still can bring to my defense that the 
same approach I picked is used in a number of test files and some other places 
and my main objective was to keep a low profile since I 
was attempting a risky change. I obviously failed with that objective as this 
bug report indicates ;-) 

I would be more than willing to rework it, now that bulk of my changes have been 
"tested by fire" (no pun intended). Still, I insist that it's 
not the cause of this crash.

I also looked into the debugger and it seems like Mozilla goes down while trying 
create a string bundle with an empty url pointer, 
specifically on the line 

http://lxr.mozilla.org/seamonkey/source/xpinstall/src/nsInstall.cpp#1236

Hope this helps & have a good weekend everyone!
J.

P.S. sgehani, I'm assigning it to you for now.

Comment 8

[Samir Gehani]

Looks like the nsIFile landing contaminated this.  Aaaargh!  <explitive>  Will 
take it.

jbetak,
Thanks for investigating.

Comment 9

[leger]

Can PDT get more info on this bug to determine PR2 blocker status?  Thanks!

Comment 10

[Samir Gehani]

This is an XPInstall API.  Anyone who clicks on/triggers a .xpi file in teh 
browser may potentially run into this crash if the .xpi's installs script called 
this API.  

Comment 11

[leger]

Putting on [nsbeta2-] radar. Not critical to beta2. Not need for Seamonkey 
beta2.

Comment 12

[Samir Gehani]

Yet another victim of the nsIFile landing.  Fix in hand.

Comment 13

[Samir Gehani]

Fix checked in.

Comment 14

[David Epstein]

Fixed on NT and Linux, but currently blocked from testing it on Mac (38989). 
Will test on Mac when possible and mark verified then.

Comment 15

[David Epstein]

fixed on NT, Win98, Linux, Mac,