289726 - XPInstall function Install.patch crashes

Status: VERIFIED FIXED

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

Description

[Karsten Düsterloh]

Trying to patch a file via XPInstall crashes Mozilla, eg. Mozilla 1.8b2
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050409].
Since the corresponding Talkback-Stack (TB4966826H) isn't that helpful, here's
one of my debug build seen with MSVC6:

nsCOMPtr<nsIFile>::~nsCOMPtr<nsIFile>() line 583 + 13 bytes
nsInstallPatch::~nsInstallPatch() line 207 + 11 bytes
nsInstallPatch::`scalar deleting destructor'(unsigned int 0x00000001) + 15 bytes
nsInstall::CleanUp() line 2527 + 30 bytes
nsInstall::FinalizeInstall(int * 0x047ef284) line 959
InstallFinalizeInstall(JSContext * 0x03637ac8, JSObject * 0x0360bc58, unsigned
int 0x00000000, long * 0x03731024, long * 0x047ef34c) line 778 + 12 bytes
js_Invoke(JSContext * 0x03637ac8, unsigned int 0x00000000, unsigned int
0x00000000) line 1293 + 23 bytes
js_Interpret(JSContext * 0x03637ac8, unsigned char * 0x03730ea3, long *
0x047efdc4) line 3566 + 15 bytes
js_Execute(JSContext * 0x03637ac8, JSObject * 0x0360bc58, JSScript * 0x03730da8,
JSStackFrame * 0x00000000, unsigned int 0x00000000, long * 0x047eff18) line 1523
+ 19 bytes
JS_EvaluateUCScriptForPrincipals(JSContext * 0x03637ac8, JSObject * 0x0360bc58,
JSPrincipals * 0x00000000, const unsigned short * 0x0372e7b8, unsigned int
0x000002db, const char * 0x00000000, unsigned int 0x00000000, long * 0x047eff18)
line 3739 + 25 bytes
JS_EvaluateUCScript(JSContext * 0x03637ac8, JSObject * 0x0360bc58, const
unsigned short * 0x0372e7b8, unsigned int 0x000002db, const char * 0x00000000,
unsigned int 0x00000000, long * 0x047eff18) line 3717 + 35 bytes
JS_EvaluateScript(JSContext * 0x03637ac8, JSObject * 0x0360bc58, const char *
0x036abbe8, unsigned int 0x000002db, const char * 0x00000000, unsigned int
0x00000000, long * 0x047eff18) line 3684 + 33 bytes
RunInstallOnThread(void * 0x0368ae70) line 558 + 30 bytes
_PR_NativeRunThread(void * 0x03691000) line 436 + 13 bytes
pr_root(void * 0x03691000) line 116 + 13 bytes
_threadstartex(void * 0x0368b078) line 212 + 13 bytes
KERNEL32! 77e8758a()

After some debugging, I found that nsInstallPatch::mPatchedFile gets destructed
too often, because it is used (in three places) to initialise another XPCOM
pointer, eg
<http://lxr.mozilla.org/mozilla/source/xpinstall/src/nsInstallPatch.cpp#248>:

    mInstall->GetPatch(&ikey, getter_AddRefs(fileName));

Alas!, GetPatch does no addreffing...

(BTW: this crashes even in Mozilla 1.0! I suspect it will even break all the way
back since bug 24984 was fixed in 2000-04-21...)

Comment 1

[Karsten Düsterloh]

Attachment: Testcase-XPI

This XPI demontrates the crash: it puts the file empty.js into the user profile
and tries to patch it.

Comment 2

[Karsten Düsterloh]

Attachment: Make GetPatch addref as expected

This patch makes sure that GetPatch does the addreffing that all callers are
expecting (mPatchList may have no entries!).

Comment 3

[Daniel Veditz [:dveditz]]

The world has changed a lot since we last tried to get patching working. I
wouldn't be at all surprised that cruft was added over time to keep the compiler
from complaining too much that isn't at all the right thing to do.

Comment 4

[Daniel Veditz [:dveditz]]

Interesting, I didn't know the QA contact field could be blank.

Comment 5

[Daniel Veditz [:dveditz]]

Comment on attachment 180215 [details] [diff] [review]
Make GetPatch addref as expected

r/sr=dveditz to stop the pain

Comment 6

[Asa Dotzler [:asa]]

Comment on attachment 180215 [details] [diff] [review]
Make GetPatch addref as expected

a=asa

Comment 7

[[e-mail]]

Fix checked in.

Comment 8

[Karsten Düsterloh]

v with 2005-04-11-06 Seamonkey trunk nightly