20777 - PlatformInit return value unchecked, failure => crash on half-initialized data structures

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P2)

Description

[Brendan Eich [:brendan]]

Nuff said, I hope -- real-world case: I was debuggging on lchiang's machine in a
mozilla tree remote-mounted from bienvenu's machine -- no component.reg and no
permissions to create it lead rapidly to badness.

/be

Comment 1

[Suresh Duddi (gone)]

We got to fix this before beta.

Comment 2

[leger]

Adding "crash" keyword to all known open crasher bugs.

Comment 3

[Suresh Duddi (gone)]

yours...

Comment 4

[Scott Collins]

Attachment: Here's the first part of the fix... now to see if the right thing happens downstream from here.

Comment 5

[Scott Collins]

Since this bug is related to the behavior discussed in bug #9120.

Comment 6

[Suresh Duddi (gone)]

The patch needs to change. Here is the deal.

PlatformInit() initializes the registry. If it fails, component manager should 
still work but just not use the registry.

So the general fix would be to protect any place we use mRegistry in some form 
or another.

Comment 7

[Suresh Duddi (gone)]

Dan veditz seems to be covering something similar. Do you want this bug dan ?

Comment 8

[Daniel Veditz [:dveditz]]

Sure... I could take it if scc wants to pass it along.

Note that many parts of the product would not run in this case. We could stick 
an autoreg in there if the registry were empty and at least build the memory 
CID/ProgID structures, but appshell components wouldn't work, the mail 
importers wouldn't work, the character set converters wouldn't work, etc. All 
those things put data elsewhere in the registry during SelfRegister. They would 
either fail to self-register at all, or simply wouldn't get called later.

I guess XPCOM shouldn't care if the registry is inaccessible. The program that 
embeds XPCOM (i.e. the Browser) will care though.

Comment 9

[Scott Collins]

Sorry for the delay.  Yes, I would love to pass it along.  Thanks for the help, 
Dan :-)

Comment 10

[David Krause]

How about a nice little error message if no component.reg stating that the user
needs to run as root once first to create it instead of just failing to start?

Comment 11

[Daniel Veditz [:dveditz]]

And where would we print the error message, given that our UI rendering stuff 
isn't loaded yet? We might get away with printing to stderr on Unix, but 
there's no console-ish thing on Mac or Windows. Then again, this is mostly a 
Unix problem so maybe that's good enough.

Reassigning to default XPCOM owner.

Comment 12

[timeless]

Windows does have a console. And as long as you don't shread it you can use it.
Windows NT also has an event log. Since the assumption is that you don't have 
privs writing to the event log makes more sense.

MacOS? Is it very taboo to use a native crash dialog?

Comment 13

[Brendan Eich [:brendan]]

I filed this crash bug a long time ago.  Is it still a crash bug?  If so, why
has it not been fixed?

/be

Comment 14

[Ray Whitmer]

This was fixed quite a while back...  Where was this bug report languishing?  
There must be a dup, but I don't know the number.  Marking fixed.

Comment 15

[David Krause]

This isn't fixed.  It's the same as brendan said 9 months ago.  No component.reg
and no permissions to create still lead to badness, i.e. segfault on startup.
Now I understand that for mozilla to work properly it really needs this file, so
I recommend either marking WONTFIX or implementing my suggestion of an error
message.

Dan, I think it would be just fine to output an error message to the stderr
console.  Although like you said it won't help the people on windows or mac, the
majority of people who would experience this bug would be on unix and more than
likely be launching from a console.

Comment 16

[Ray Whitmer]

This bug, as stated, is that PlatformInit does not check the return value and 
causes a crash.  This has been fixed, almost identically to the patch attached 
here.  Please, look at the code and verify this.  You may be confusing this bug 
with 9120, which is stated more-generally.  Or open a new bug if you need one, 
that describes some new problem to be solved.

Marking fixed.  Your reason for reopening does not relate to the description of 
this bug.

Comment 17

[timeless]

That's interesting, because the bug as stated is /be's description of the bug, 
not the summary. Fix the bug, if necessary while working on pieces change the 
summary to reflect the current problem. Or if you feel it is fixed then file a 
new bug, cc everyone who cares and explain where others should go to check on 
it.

Comment 18

[Brendan Eich [:brendan]]

Open a new bug if the specific cause of this one has been fixed (I believe rayw) 
and the same symptom is now reproducing due to a different cause.  Timeless, 
this means you (or if not you, then whoever can repro the symptom).

Often we file bugs based on symptoms, then split them according to causes as we 
diagnose.  That didn't happen here yet, but it should (summary trumps symptom 
statement in the initial descriptiong -- I put a precise, cause-specific summary 
in this bug on purpose).

/be