204201 - Add PR_GetLogModule

Status: NEW

(NSPR :: NSPR, enhancement)

Description

[timeless]

I have managed to get XPCOM shutdown to include PR_Cleanup.
However NSPR Logging code is causing me a small problem:
Some xpcom code (nsThread::Exit) is called by PR_Cleanup because it's the
cleanup method for the nspr thread.
nspr doesn't allow you to change the thread private destructor without it being called.

This means that something like this happens:
nspr inits
xpcom inits
logging inits
xpcom creates a thread and a prlog for threads
...
xpcom starts shutting down
nspr starts shutting down
nspr deletes its logging modules
nspr deletes its threads
 xpcom's nsThread::Exit is called
  this wants to use its static pointer to log some output, which would be fine
  for non main threads. but for the main thread the log was already deleted and
  the point nsIThreadLog was left dangleing because there was no way for nspr
  to delete it.

  things that could happen here:
   app can use deleted memory to start a logging call (crash)
   app might trigger nspr init

app quits

I see two ways to avoid this problem:
1. NSPR provides a PR_GetLogModule function which will not initialize nspr.
2. NSPR provides a PR_TagLogModule(PRLogModule *module, PRLogModule **reference)
    in this case, nspr would null out reference when it deletes the logmodule.

I chose option 1 because it's a single tiny safe function which doesn't bloat
the log module data.

Comment 2

[timeless]

Attachment: example use of PR_GetLogModule

Comment 3

[timeless]

Attachment: implement PR_GetLogModule

whoops. wrong equality test.

Comment 4

[timeless]

Comment on attachment 122304 [details] [diff] [review]
example use of PR_GetLogModule

darin: this is the magic i was talking about.
(specifically the thread stuff)

Note that NSPR owns and will release the last refcount of the main thread as
part
of PR_Cleanup, which is why the assertion's expected value is changed.

There are two other changes in this patch (process pending events - which i've
discussed w/ you, and memory - which is something i believe i've mentioned to
dougt)

Comment 5

[Darin Fisher]

there has to be a better solution to this issue.  i don't think querying for the
log module each time you need it is the best solution.

also, i'm not very interested in having XPCOM call PR_Cleanup.  i don't see the
value in doing so.  it seems to cause more problems than its worth.  what's the
real value in having PR_Cleanup run?  (and don't say to reduce purify noise!)

Comment 6

[timeless]

the value is that xpcom no longer has to have an evil hack which forces the main
thread to be deleted:
@@ -431,12 +442,9 @@
 nsThread::Shutdown()
 {
     if (gMainThread) {
-        // XXX nspr doesn't seem to be calling the main thread's destructor
-        // callback, so let's help it out:
-        nsThread::Exit(NS_STATIC_CAST(nsThread*, gMainThread));
         nsrefcnt cnt;
         NS_RELEASE2(gMainThread, cnt);
-        NS_WARN_IF_FALSE(cnt == 0, "Main thread being held past XPCOM shutdown.");
+        NS_WARN_IF_FALSE(cnt == 1, "Main thread being held past XPCOM shutdown.");

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.