235691 - Mozilla/Firefox don't start

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Sergei Dolgov]

Especailly if run from dist/bin.
Tested with 2004-02-24 nightly sources
See comments here:
http://www.livejournal.com/community/bezilla/24245.html

Comment 1

[Patrick]

please take a look at bug 235734

Comment 2

[Sergei Dolgov]

bug 235734 redirects to bug  24804

Comment 3

[Sergei Dolgov]

mistypo. Should be bug 234804

Comment 4

[Patrick]

Not quite, it redirects to bug 234804 ;)
Sorry, i accidentally took one of todays dupes instead of the original. Anyways,
the original bug deals with recent build problems.

PS: your bug is a bit sketchy on details. Please use the Bugzilla helper next
time, and/or provide all steps you take and any & all warning messages you get.

Comment 5

[Sergei Dolgov]

Unfortunately myself i'm unable to give more details. In attempt to run it after
build process from dist/bin it just quits. I tried recently it with 1.7a release
sources from 19. Feb. 2004 - same sad result.
So, i suspect, big bustage happened 3-4 Feb 2004 for all platforms wasn't fixed
for BeOS at all.
Though, there are more details from Simon's investigation - 
"
I've been peppering the startup code with printf's, I'm beginning to get an idea
of where it fails.

Early on in the start up procedure, XPCOM tries to create an instance
(do_CreateInstance) of something with a contract ID of
"@mozilla.org/embedcomp/appstartup-notifier" - mozilla can't find this
particular "factory" and closes."

Comment 6

[Simon]

Sorry for probably using the completely incorrect terminology in the bit of
reporting Sergei quoted.

It seems to be very closely related to the current directory you are in.

If you run "mozilla" for "dist/bin/", this is the output I get from my printfs:
----
$ mozilla
XPCom_glue
get GRE path
sgrloc: .
Loaded stub: ./libxpcom.so.stub!
get GRE path
Starting Directory Service
Init nsDirectoryService
Init nsDirectoryService success!
Creating compMgr
Failed to find directory for key: MozBinD
Failed to find directory for key: MozBinD
get GRE path
Failed to find directory for key: MozBinD
Failed to find directory for key: MozBinD
aContractID: @mozilla.org/file/directory_service;1
Failed to find directory for key: MozBinD
aContractID: @mozilla.org/supports-array;1
get GRE path
GRE_startup done!
Init Event Service
aContractID: @mozilla.org/event-queue-service;1
aContractID: @mozilla.org/observer-service;1
aContractID: @mozilla.org/embedcomp/appstartup-notifier;1
Hash contract ID: @mozilla.org/embedcomp/appstartup-notifier;1
FACTORY NOT REGISTERED!
---BANG - executable quits ---

Running from any other directory with a relative path gives an error even
quicker. Eg from the root of the mozilla tree:
----
$ dist/bin/mozilla
XPCom_glue
get GRE path
sgrloc: dist/bin
Couldn't load stub!
---BANG - quits here ---

I've just discovered now that running with the absolute path actually works:
----
$ /boot/home/mozilla/dist/bin/mozilla
XPCom_glue
get GRE path
sgrloc: /boot/home/mozilla/dist/bin
Loaded stub: /boot/home/mozilla/dist/bin/libxpcom.so.stub!get GRE path
Starting Directory Service
Init nsDirectoryService
Init nsDirectoryService success!
Creating compMgr
Failed to find directory for key: MozBinD
Failed to find directory for key: MozBinD
get GRE path
Failed to find directory for key: MozBinD
aContractID: @mozilla.org/file/directory_service;1
Failed to find directory for key: MozBinD
GRE_startup done!
Init Event Service
aContractID: @mozilla.org/event-queue-service;1
aContractID: @mozilla.org/observer-service;1
aContractID: @mozilla.org/js/jsd/app-start-observer;2
Loaded stub: /boot/home/mozilla/dist/bin/components/libjsd.so.stub!aContractID:
@mozilla.org/js/jsd/debugger-service;1
aContractID: @mozilla.org/js/xpc/RuntimeService;1
Loaded stub: /boot/home/mozilla/dist/bin/components/libxpconnect.so.stub!
aContractID: @mozilla.org/js/xpc/ContextStack;1
aContractID: @mozilla.org/supports-array;1
aContractID: @mozilla.org/moz/jsloader;1
aContractID: @mozilla.org/embedcomp/appstartup-notifier;1
Loaded stub: /boot/home/mozilla/dist/bin/components/libembedcomponents.so.stub!
aContractID: @mozilla.org/typeaheadfind;1
Loaded stub: /boot/home/mozilla/dist/bin/components/libtypeaheadfind.so.stub!
aContractID: @mozilla.org/preferences-service;1 
[snip]
----build runs fine---

Comment 7

[Simon]

Caused by changes to the mozilla startup script:
http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/xpfe/bootstrap/mozilla.in

I found a mozilla script from before those changes, and that starts the build 
fine.

Comment 8

[Simon]

Adding "exec" before the command to launch mozilla in the startup "mozilla"
script fixes the bug.

However, the comments in http://bugzilla.mozilla.org/show_bug.cgi?id=233153
suggest "exec" is not a prefered solution. Could we have a conditional for BeOS
to solve this? Startup/Ending scripts are not really an important issues for us,
but having a browser that works certainly is!

Comment 9

[Sergei Dolgov]

I think your last comment should be put in bug with patch killed BeOS port an
those questions adressed to author of patch and reviewer.

They may have cleaner look at the problem.

Maybe some bugzilla dependency between two bugs must be set

Comment 10

[Sergei Dolgov]

Also, it seems, according to Simon's investigation, bug component must be
chamged to Build Config from XPCOM

Comment 11

[Simon]

Adding exec is not a complete fix - if you are in the dist/bin directory and
type "mozilla" then it works, but just double clicking on mozilla in the file
manager doesn't work (it follows the symlink and runs the script in the
/mozilla/xpfe/bootstrap directory). I'm not sure that double clicking on the
file in dist/bin ever worked in fact.

Really, we would ideally like to get rid of the startup script altogether, as no
programs on BeOS use startup scripts.

Comment 12

[Disreali]

I beleive that this bug has been fixed. Please confirm.

Comment 13

[Sergei Dolgov]

Bug is still here with latest sources.
Start form dist/bin of ./mozilla scripts quits immediatelly
Start with tqh patches for ports in plevent.c and nsAppShell.cpp
shows for moment mozilla logo window and then quits immediately

Comment 14

[tqh]

The patch fyysik mentions is bug 274281. (It's ready for checkin if the trunk is
open now).

Comment 15

[Sergei Dolgov]

pair
 ./run-mozilla.sh ./mozilla-bin
lacks realpath() for dirname

exec seems supplying that with overwriting argv[0]

Comment 16

[Sergei Dolgov]

Fredrik's idea about nsDirectory service was right.
With his private patch all starts from script without  "exec".
Fredrik, can you submit patch as soon as possible, with explanations?

Comment 17

[tqh]

I only have AVIARY-branch patches right now. The problem is that under Mozilla
xpcom is initialized with null for Mozilla binary directory. The code for
getting the directory when null was passed in was bad
(nsDirectoryService::GetCurrentProcessDirectory). It tried to get the
'MOZILLA_FIVE_HOME' env variable as first way. It returned '.' which didn't get
resolved correctly to an absolute path. It's much better to just get the
absolute path from the apps image, and strip of the applications name.

#elif defined(XP_BEOS)
      static char buf[MAXPATHLEN];
      int32 cookie = 0;
      image_info info;
      char *p;
      *buf = 0;
      if(get_next_image_info(0, &cookie, &info) == B_OK)
      {
        strcpy(buf, info.name);
        if((p = strrchr(buf, '/')) != 0)
        {
          if( (p-2 >= buf) && *(p-2)=='/' && *(p-1)=='.')
            p -=2; 
          *p = 0;
          localFile->InitWithNativePath(nsDependentCString(buf));
          *aFile = localFile;
          return NS_OK;
        }
    }
#endif

Comment 18

[Christian :Biesinger (don't email me, ping me on IRC)]

>          if( (p-2 >= buf) && *(p-2)=='/' && *(p-1)=='.')
>            p -=2;

tqh: why are these lines needed?

Comment 19

[tqh]

They arn't but I just think it looks ugly with paths that look like
dir1/dir2/./components which is the result otherwise under BeOS.

Comment 20

[tqh]

Attachment: Patch to always get process dir from app image instead of env. variable

Comment 21

[Sergei Dolgov]

tqh, what are you plans about r and sr requests?
Is this patch ready in your opinion?

Comment 22

[tqh]

I had forgot about this one, and thought it was already in tree. I think that
patch is ok.

Comment 23

[tqh]

Comment on attachment 170402 [details] [diff] [review]
Patch to always get process dir from app image instead of env. variable

r?
Basically just remove the look at the env variable. And remove ./ when the path
is /./

Comment 24

[Sergei Dolgov]

Comment on attachment 170402 [details] [diff] [review]
Patch to always get process dir from app image instead of env. variable

r=sergei_d
Works nicely.
No side-effects noticed.
Urgent.

Comment 25

[tqh]

Comment on attachment 170402 [details] [diff] [review]
Patch to always get process dir from app image instead of env. variable

sr?
(and if possible checkin)

Comment 26

[Doug Turner (:dougt)]

Comment on attachment 170402 [details] [diff] [review]
Patch to always get process dir from app image instead of env. variable

1) image_info.name is never longer than MAXPATHLEN?
2) What if after (p = strrchr), p=(buf+1).  p-2 is undefined.
3) In the patch you added an extra line of ws.
4) why does buf have to be static?

Comment 27

[tqh]

1) That is the code in CVS, so I didn't think to improve that. 
2) p-2 >= buf checks that p-2 is not pointing to before the start of the buf
address. Note the difference between p and *p, or am I missing something?
3) Sneaked in in editing, sorry.
4) Dunno, it's like that in CVS.

Comment 28

[tqh]

Regarding 1):
typedef struct { 
         image_id id; 
          image_type type; 
         int32 sequence; 
         int32 init_order; 
         B_PFV init_routine; 
         B_PFV term_routine; 
         dev_t device; 
         ino_t node; 
          char 
name[MAXPATHLEN];
          void *text; 
          void *data; 
         int32 text_size; 
          int32 data_size; 
      } image_info

So image_info.name is at most maxpathlen.

Comment 29

[tqh]

Attachment: changed the static buf, and improved formatting inside ifdef BEOS

Updated formatting and removed static, r?
The p-=2 will never be smaller than buf as the if checks (p-2 >= buf )
The image_info.name is defined as char name[MAXPATHLEN].

Comment 30

[Sergei Dolgov]

tqh, i'm again unsure about formatting
1)There are reminds of old code which used tabs spaces, e.g. line with
strcpy(buf, info.name), new uses spaces

I wish to know from beauty gurus what that headers means:
/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
namely, "indent-tabs-mode" - does it mean that we are allowed to use tabs, of
so, why spaces are used overall in code? Doug ?

2)if i understand, you removed "block" if {}else {} but it looks like it still
has extra identation from that "else {}" part. 10 spaces before *p = 0; wile it
must be 4, 8, 12, or 1/2/3 tabs.

Comment 31

[tqh]

Sergei, I tried to match coding style of the rest of the code, and most other
files. It's unfortunate that BeIDE is a bit different.

Comment 32

[Sergei Dolgov]

tqh, it is not about just your misformatting, but i'm confused overall with this
situation, about formatting rules declared in beginning of file, and actual
formatting.
So i'm afraid if i put (functionality) review, it can be reject at sr level again.
At moment i have problems with my cvs settings, to checkout this original file,
but i will do it today anyway.

Comment 33

[Sergei Dolgov]

Comment on attachment 171986 [details] [diff] [review]
changed the static buf, and improved formatting inside ifdef BEOS

r=sergei_d
For now all spaces look to be on proper place and in proper amount

Comment 34

[tqh]

Comment on attachment 171986 [details] [diff] [review]
changed the static buf, and improved formatting inside ifdef BEOS

Updated formatting and removed static, sr?
The p-=2 will never be smaller than buf as the 'if' checks (p-2 >= buf )
The image_info.name is defined as char name[MAXPATHLEN].

Comment 35

[Doug Turner (:dougt)]

Comment on attachment 171986 [details] [diff] [review]
changed the static buf, and improved formatting inside ifdef BEOS

thanks for the comments.

Comment 36

[Christian :Biesinger (don't email me, ping me on IRC)]

Checking in nsDirectoryService.cpp;
/cvsroot/mozilla/xpcom/io/nsDirectoryService.cpp,v  <--  nsDirectoryService.cpp
new revision: 1.75; previous revision: 1.74
done

please mark fixed if it is...

Comment 37

[Niels Reedijk]

Checked in by biesi.

Comment 38

[Sergei Dolgov]

Patch fixed startup problem,
but another problem is still here - without "exec" (realpath ?)
stack crawls are unusable - no names there, only binary addresses