354862 - xpicleanup crashes on OS/2

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Steve Wendt]

If an xpicleanup.dat file is present, Seamonkey 1.1a won't start.  Attempting to run xpicleanup manually yields a crash:

Killed by SIGSEGV
pid=0x0084 ppid=0x0044 tid=0x0001 slot=0x006a pri=0x0200 mc=0x0001
D:\OS2APPS\MOZILLA\BIN\XPICLEANUP.EXE
XPICLEAN 0:0000046e
cs:eip=005b:0001046e      ss:esp=0053:0011fde0      ebp=0011ffa0
 ds=0053      es=0053      fs=150b      gs=0000     efl=00012246
eax=00000000 ebx=0011ffa8 ecx=00000000 edx=0011ffc5 edi=0011fe08 esi=00000000
Process dumping was disabled, use DUMPPROC / PROCDUMP to enable it. 

There is speculation that there is a bug in the libc06 spawn/exec*() implementation.  Another user reports that it works on trunk build from 2 days ago; not sure if that is a libc05 or lib06 build.

Setting initial state to unconfirmed, as I have yet to see anyone else confirm that they see this.

Comment 1

[Andy Willis (abwillis)]

Just for the record, it is a libc06 trunk build that I was referring to.  

Comment 2

[Peter Weilbacher]

Yes, the same crash happens here, but that doesn't look like it can be caused by the libc06 implementation of spawn/exec*().

From the address it seems to crash in PerformScheduledTasks().

Comment 3

[Peter Weilbacher]

It does not crash when called as ".\xpicleanup" instead of just "xpicleanup". The logic to strip off the path in InstallCleanupOS2.cpp is flawed.

But I think the real problem is that xpicleanup does not get called correctly from SeaMonkey, and not that it crashes when called in a wrong way.

Comment 4

[Peter Weilbacher]

Attachment: Correct crash when backslash is missing

This adds another safeguard to xpicleanup in case there is no backslash in the path. It includes the same change in the Windows code, but as I don't have Windows I cannot test and don't know if it is even necessary.

Comment 5

[Peter Weilbacher]

Just for the record, the same crash occurs on trunk (and probably on Windows), too, and it has nothing to do with PerformScheduledTasks().

Comment 6

[Mike Kaply [:mkaply]]

Comment on attachment 240718 [details] [diff] [review]
Correct crash when backslash is missing

r=mkaply for the OS/2 part only

Comment 7

[Daniel Veditz [:dveditz]]

Comment on attachment 240718 [details] [diff] [review]
Correct crash when backslash is missing

sr=dveditz (and r= on the windows patch)

Comment 8

[Peter Weilbacher]

Thanks for the reviews. Fixed on trunk.

Comment 9

[Peter Weilbacher]

Comment on attachment 240718 [details] [diff] [review]
Correct crash when backslash is missing

I think we should finally get this into branch, too.   At least for OS/2 this is an annoying problem and the fix hasn't caused a problem in the last 10 months.

Comment 10

[Daniel Veditz [:dveditz]]

Comment on attachment 240718 [details] [diff] [review]
Correct crash when backslash is missing

approved for 1.8.1.7, a=dveditz for release-drivers

Comment 11

[Peter Weilbacher]

Verified as fixed on OS/2 with the SeaMonkey 2007-08-31 branch nightly. Leaving as fixed1.8.1.7 for Windows verification.

Comment 12

[Carsten Book [:Tomcat]]

Peter: Can you verify this Bug also for 2.0.0.8, since i don`t have a OS/2 System ?

Comment 13

[Peter Weilbacher]

That's what  I said in comment 11. :-) Assuming that Carsten's comment means that verification was done on Windows, I now mark it verified.