58541 - set NSPR_LOG_MODULES=nsComponentManager:5 in the environment crashes mozilla at startup

Status: RESOLVED WONTFIX

(Core :: XPCOM, defect, P3)

Description

[Luke]

I have

set NSPR_LOG_MODULES=nsComponentManager:5

as part of my build config.

When I start mozilla, it crashes at startup in xpcom.dll.
I can't get a stack trace, because if I load mozilla using
the debugger, it doesn't crash.

Comment 1

[Axel Hecht]

Here my research, copied from a mail to warren:

In nsLoggingService::Init, the environment variable NSPR_LOG_FILE is
read, and the log is opened.

in nsLogging.cpp:183, outputPath is set to the scope local variable
nspr_log_file, so in line 216, there is garbage in it, it goes to
error:, clears the gLogMonitor, and I'm doomed for the rest. That is, when
opening the log, the gLogMonitor is null.

It should be fixed, if we directly set outputPath in 179, instead of using the
local variable nspr_log_file.

I may test that later.

Btw, that error seems to be there since rev 1.1 :-?

Axel

Comment 2

[Axel Hecht]

I will submit a patch to fix this bug.
It depends on the patch for http://bugzilla.mozilla.org/show_bug.cgi?id=58514
But then I can do logging allright. Yep.

warren, r=?, sr by scc?, 58514 misses a sr, too. Should go in together, IMHO.

Axel

Comment 3

[Axel Hecht]

Attachment: patch to fix NSPR_LOG_FILE, includes patch from 58514

Comment 4

[Axel Hecht]

Attachment: updated fix

Comment 5

[Axel Hecht]

Hi,
as the fix for 58514 is in, here is the updated patch.
How about a review?

Axel

Comment 6

[Henrik Gemal]

what's happening of this one...?
if the fix is still valid, get a review using
http://www.mozilla.org/hacking/reviewers.html

Comment 7

[Axel Hecht]

Hi Henrik,
the patch should still be valid, I poked scc a few times, he's got a mail about
this in his inbox, what's more to do? Yes, poke again. Thanx :-).

Axel

Comment 8

[Axel Hecht]

Hi brendan,
care to take this one off of scc's broken hands? It's a half-a-brain 4 line
patch.

Thanks,

Axel

Comment 9

[Brendan Eich [:brendan]]

The fact that nspr_log_file is block-local means nothing, because the pointer it
contains is either null, or a valid pointer to a static string in the
environment passed on the stack by the kernel when the process was exec'd.  So I
don't understand the bug, but the patch still looks ok to me, because it
eliminates an unnecessary variable.

Hope I'm not just sleepy -- can someone enlighten me as to what bug this patch
is fixing?

/be

Comment 10

[Axel Hecht]

Hi Brendan,
I'm on solaris, compiling with gcc. It used to free my buffer the moment I
leave the scope of nspr_log_file, thus giving me a segmentation fault.
I tried to reproduce this error today, without success. But this may be because
I couldn't make mozilla write something to a logfile. It openend allright the
logfile allright, but no content. IIRC, this crasher appeared while writing the
log. Wow, it's been a while.
Could we get this in anyway? I don't have time shortly to do a logging testcase.

Axel

Comment 11

[Brendan Eich [:brendan]]

What buffer gets freed?  As I wrote, the returned pointer from getenv points to
stack storage that lives as long as the process lives.

But the gratuitous variable elimination (nspr_log_file is unnecessary) works for
me, so go ahead and check in.  I just don't believe this will cure any crash bug
unless you have a broken compiler.

/be

Comment 12

[Doug Turner (:dougt)]

to default.

Comment 13

[Axel Hecht]

die, XPCOM logging, die.
nsLogging.cpp was removed, see
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=mozilla%2Fxpcom%2Fbase&file=nsLogging.cpp&filetype=match&who=&whotype=match&sortby=Who&hours=2&date=explicit&mindate=11%2F1%2F2001&maxdate=1%2F1%2F2002&cvsroot=%2Fcvsroot
setting resolution to WONTFIX, as the code is still buggy in the Attic.