Opened 11 years ago

Closed 11 years ago

#1620 closed bug (fixed)

Firefox crashes with "-safe-mode" switch

Reported by: umccullough Owned by: axeld
Priority: normal Milestone: R1
Component: System/libroot.so Version: R1/pre-alpha1
Keywords: Cc: tqh, mjw
Blocked By: Blocking:
Has a Patch: no Platform: x86

Description

The attached backtrace shows a crash when attempting to start Firefox 2.0.0.10pre (R5 net_server version) using the -safe-mode commandline switch using Haiku hrev22903 on a PIII 600 w/512mb RAM.

Without the -safe-mode switch, Firefox seems to load fine and can be used for a short period of time before the app_server appears to freeze.

I used an empty profile from my R5 machine when testing.

Attachments (2)

ff safe-mode crash (4.1 KB) - added by umccullough 11 years ago.
backtrace of firefox safe-mode crash
Application.cpp.patch (354 bytes) - added by mjw 11 years ago.

Download all attachments as: .zip

Change History (14)

Changed 11 years ago by umccullough

Attachment: ff safe-mode crash added

backtrace of firefox safe-mode crash

comment:1 Changed 11 years ago by tqh

As we don't seem to do anything with ArgvReceived (in this 2.0.X branch): http://lxr.mozilla.org/mozilla1.8.0/source/toolkit/xre/nsNativeAppSupportBeOS.cpp#56

It must be related to how Firefox handles it and BApplication's ArgvReceived: http://lxr.mozilla.org/mozilla1.8.0/source/toolkit/xre/nsAppRunner.cpp#1894

I can't see anything except that argv is passed straight thru to BApplication's method.

Maybe we should backport some code from trunk: http://lxr.mozilla.org/seamonkey/source/toolkit/xre/nsNativeAppSupportBeOS.cpp#113

comment:2 Changed 11 years ago by tqh

Seems to be related to that switch only, so I wonder what Firefox does with that case.

comment:3 Changed 11 years ago by mjw

I have been working on freeciv and have noticed the same problem.

What happens in freeciv's case is that some elements of argv are set to NULL before the BApplication object is instantiated. argc is unchanged. Eventually, BApplication::_ArgvReceived() is called with the correct value of argc, but all the NULL elements of argv are missing (so argc > num elements in argv).

So, to cut a long story short, the free(argv[i]) call in _ArgvReceived() ends up freeing random pointers, causing the crash.

I have attached a patch that fixes this bug.

Changed 11 years ago by mjw

Attachment: Application.cpp.patch added

comment:4 Changed 11 years ago by mjw

Cc: mjw added

comment:5 Changed 11 years ago by stippi

The patch does not fix the error completely. The entire array needs to be initialized to NULLs, since an error could occur in the middle of loop. Then your patch would only initialize that entry, but break out of the loop early. Then the remaining items would still be uninitialized. But thanks for catching this one! (And sorry I forgot to credit you in the commit message, didn't intend to!)

Does hrev25494 also fix the Firefox problem?

comment:6 Changed 11 years ago by mjw

Oh yeah - missed that one. D'oh. Thanks.

comment:7 Changed 11 years ago by umccullough

I just tested firefox 2.0.0.15pre BONE (experimental build from bebits) on hrev25493 and the problem didn't even exist there, so it may have already been fixed sometime in the last 6 months.

Wish I'd noticed earlier - I'll try to track down the approximate timeframe that it got fixed if I can.

comment:8 Changed 11 years ago by korli

then it's fixed ?

comment:9 Changed 11 years ago by umccullough

It worked prior to hrev25494 (in fact, I tested back to 24500 and it even worked there I believe)

With hrev25536 now though, it spits some output to the terminal window that I hadn't seen before:

/boot/apps/firefox> firefox --safe-mode Error parsing B_ARGV_RECEIVED message. Message: BMessage('_ARG') {

argc = int32(0x2 or 2) argv = string("./firefox", 10 bytes) cwd = string("/boot/apps/firefox", 19 bytes)

} /boot/apps/firefox>

This was tested with Firefox 2.0.0.15pre BONE from the Experimental Build page on bebits.

I think this issue can safely be closed.

comment:10 Changed 11 years ago by umccullough

Just in case it wasn't clear - even with the output mentioned in my previous comment, it does start without crashing at least.

comment:11 Changed 11 years ago by umccullough

Someone please close! :)

comment:12 Changed 11 years ago by mmlr

Resolution: fixed
Status: newclosed

Fixed apparently.

Note: See TracTickets for help on using tickets.