391174 - PR_Initialized() returns true in the middle of initialization

Status: RESOLVED WONTFIX

(NSPR :: NSPR, defect)

Description

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

PR_Initialized() returns true even when NSPR initialization is not complete.  I think it should return false if initialization is incomplete; failing that, there should at least be an API that NSPR clients can use to determine if initialization is complete.

This is a problem for me because I'm trying to get trace-malloc, which is a memory debugging tool that overrides malloc, free, etc., to use NSPR thread-local storage.  I have code that does the following:

    if (!tracing_enabled || !PR_Initialized() ||
        (t = get_tm_thread())->suppress_tracing != 0) {
        ...

but this is crashing (due to a PR_ASSERT) within get_tm_thread (my function that gets the appropriate thread-local object) because NSPR is not fully initialized:

#0  0x00110402 in __kernel_vsyscall ()
#1  0x00d68fa0 in *__GI_raise (sig=6)
    at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#2  0x00d6a8b1 in *__GI_abort () at abort.c:88
#3  0x006f5e98 in PR_Assert (s=0x719608 "NULL != _pr_tpd_destructors", 
    file=0x719548 "/builds/trunk/mozilla/nsprpub/pr/src/threads/prtpd.c", 
    ln=138) at /builds/trunk/mozilla/nsprpub/pr/src/io/prlog.c:546
#4  0x006fa447 in PR_NewThreadPrivateIndex (newIndex=0x179b88, 
    dtor=0x175a50 <free_tm_thread>)
    at /builds/trunk/mozilla/nsprpub/pr/src/threads/prtpd.c:138
#5  0x00175863 in get_tm_thread ()
    at /builds/trunk/mozilla/tools/trace-malloc/lib/nsTraceMalloc.c:255
#6  0x00177cb5 in calloc (count=1, size=20)
    at /builds/trunk/mozilla/tools/trace-malloc/lib/nsTraceMalloc.c:1115
#7  0x0072335c in _dlerror_run (operate=0x722bd0 <dlopen_doit>, 
    args=0xbfa3242c) at dlerror.c:142
#8  0x00722ba4 in __dlopen (file=0x0, mode=1375) at dlopen.c:87
#9  0x006fbc13 in pr_FindSymbolInProg (name=0x0)
    at /builds/trunk/mozilla/nsprpub/pr/src/malloc/prmem.c:130
#10 0x006fbcff in _PR_InitZones ()
    at /builds/trunk/mozilla/nsprpub/pr/src/malloc/prmem.c:186
#11 0x00701941 in _PR_InitStuff ()
    at /builds/trunk/mozilla/nsprpub/pr/src/misc/prinit.c:176
#12 0x00712768 in PR_GetCurrentThread ()
    at /builds/trunk/mozilla/nsprpub/pr/src/pthreads/ptthread.c:640
#13 0x00620097 in nsAutoOwningThread (this=0x6e7a0c)
    at ../../dist/include/xpcom/nsISupportsImpl.h:79
#14 0x006ab260 in nsTraceRefcntImpl (this=0x6e7a04)
    at /builds/trunk/mozilla/xpcom/base/nsTraceRefcntImpl.h:45
#15 0x006a8be6 in __static_initialization_and_destruction_0 (
    __initialize_p=<value optimized out>, __priority=6)
    at /builds/trunk/mozilla/xpcom/base/nsTraceRefcntImpl.cpp:1358
#16 0x006ccd36 in __do_global_ctors_aux () from ./libxpcom_core.so
#17 0x0061af6d in _init () from ./libxpcom_core.so
#18 0x00512ff3 in call_init (l=0xb7f3a000, argc=5, argv=0xbfa325c4, 
    env=0xbfa325dc) at dl-init.c:70
#19 0x00513103 in _dl_init (main_map=0x521650, argc=5, argv=0xbfa325c4, 
    env=0xbfa325dc) at dl-init.c:134
#20 0x0050584f in _dl_start_user () at rtld.c:780

Comment 1

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

That said, I can work around this by assuming that there's only one thread until the trace-malloc startup function gets called from main.  (trace-malloc is already making that assumption, actually, since that's when it allocates its locks.)

Comment 2

[Wan-Teh Chang]

Attachment: Possible patch

David, thank you for the bug report.

I looked at this problem before, and I remember the issue was that NSPR
used _pr_initialized to also mean "initialization is in progress" and
stop recursive calls to _PR_InitStuff.  If this is right, then we have
two possible fixes.

1. Find out why NSPR calls _PR_InitStuff recursively and fix that.

2. Add a new variable to indicate the "initialization is in progress"
state, which is what this patch does.

Let me know if this patch works for you.  This simple patch seems
safe, so I'd also appreciate it if you could review it.

Comment 3

[Wan-Teh Chang]

Attachment: Possible patch (corrected)

The previous patch contains an unrelated makefile change by mistake.

Comment 4

[Julien Pierre]

Wan-Teh,

As I recall that there were thread-safety issues with NSPR initialization. We had numerous requests at Sun to make it thread-safe internally, but we had concluded that it couldn't be done without huge performance hits in the implicit initialization code that affects almost every NSPR call. So, IMO, calling PR_Initialized() only makes sense in a single-thread environment, or when your code knows and can enforce that no other threads have called NSPR yet.

Comment 5

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

It turns out this wasn't the only problem I hit trying to use NSPR thread-local storage for trace-malloc:
 * NSPR could malloc inside PR_GetThreadPrivate, which required
   workarounds that cost me all of the performance advantage of
   thread-local storage
 * PR_SetThreadPrivate freed the old thread-local storage array
   before assigning the new one (I had a simple patch for this one)
 * _PRI_DetachThread freed the thread-local storage array while I
   still needed it (and I don't see a solution for this one)
So after the third problem I just used platform-specific thread-local storage APIs that don't use malloc/free.  So I don't have any need for this anymore.

That said, the patch looks fine; thanks for the quick response.

Re comment 4:  This was a single-threaded situation; I was just calling PR_Initialized from a |malloc| implementation.  That is, I was overriding |malloc| for leak debugging, so NSPR was calling my code during its own initialization.

Comment 6

[Wan-Teh Chang]

OK, I'll resolve this bug WONTFIX, until someone else
really needs this bug fixed.

It makes sense for trace-malloc to use platform-specific
functions directly.  In some sense trace-malloc is lower
layer than NSPR.