xpicleanup crashes on OS/2

VERIFIED FIXED

Status

SeaMonkey
UI Design
VERIFIED FIXED
11 years ago
10 years ago

People

(Reporter: Steve Wendt, Assigned: Peter Weilbacher)

Tracking

({verified1.8.1.8})

Trunk
x86
All
verified1.8.1.8

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

11 years ago
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.
Just for the record, it is a libc06 trunk build that I was referring to.  
(Assignee)

Comment 2

11 years ago
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().
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 3

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

Comment 4

11 years ago
Created attachment 240718 [details] [diff] [review]
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.
Assignee: jag → mozilla
Status: NEW → ASSIGNED
Attachment #240718 - Flags: superreview?(dveditz)
Attachment #240718 - Flags: review?(mozilla)
(Assignee)

Comment 5

11 years ago
Just for the record, the same crash occurs on trunk (and probably on Windows), too, and it has nothing to do with PerformScheduledTasks().
OS: OS/2 → All
Version: 1.8 Branch → Trunk

Comment 6

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

r=mkaply for the OS/2 part only
Attachment #240718 - Flags: review?(mozilla) → review+
Comment on attachment 240718 [details] [diff] [review]
Correct crash when backslash is missing

sr=dveditz (and r= on the windows patch)
Attachment #240718 - Flags: superreview?(dveditz) → superreview+
(Assignee)

Comment 8

11 years ago
Thanks for the reviews. Fixed on trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
(Assignee)

Comment 9

10 years ago
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.
Attachment #240718 - Flags: approval1.8.1.7?
Comment on attachment 240718 [details] [diff] [review]
Correct crash when backslash is missing

approved for 1.8.1.7, a=dveditz for release-drivers
Attachment #240718 - Flags: approval1.8.1.7? → approval1.8.1.7+
(Assignee)

Updated

10 years ago
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.1.7
(Assignee)

Comment 11

10 years ago
Verified as fixed on OS/2 with the SeaMonkey 2007-08-31 branch nightly. Leaving as fixed1.8.1.7 for Windows verification.
Peter: Can you verify this Bug also for 2.0.0.8, since i don`t have a OS/2 System ?
(Assignee)

Comment 13

10 years ago
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.
Keywords: fixed1.8.1.8 → verified1.8.1.8
You need to log in before you can comment on or make changes to this bug.