bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

Status

NSPR
NSPR
--
enhancement
15 years ago
12 years ago

People

(Reporter: timeless, Assigned: Wan-Teh Chang)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

15 years ago
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.
(Reporter)

Comment 1

15 years ago
Created attachment 122303 [details] [diff] [review]
(Reporter)

Comment 2

15 years ago
Created attachment 122304 [details] [diff] [review]
example use of PR_GetLogModule
(Reporter)

Updated

15 years ago
Attachment #122303 - Flags: review?(wtc)
(Reporter)

Updated

15 years ago
Attachment #122303 - Attachment description: implement PR_GetLogModule →
(Reporter)

Comment 3

15 years ago
Created attachment 122305 [details] [diff] [review]
implement PR_GetLogModule

whoops. wrong equality test.
Attachment #122303 - Attachment is obsolete: true
(Reporter)

Updated

15 years ago
Attachment #122303 - Flags: review?(wtc)
(Reporter)

Updated

15 years ago
Attachment #122305 - Flags: review?(wtc)
(Reporter)

Comment 4

15 years ago
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)
Attachment #122304 - Flags: review?(darin)
(Reporter)

Updated

15 years ago
Blocks: 207668

Comment 5

15 years ago
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!)

Updated

15 years ago
Attachment #122304 - Flags: review?(darin)
(Reporter)

Comment 6

15 years ago
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.");
QA Contact: wtchang → nspr
You need to log in before you can comment on or make changes to this bug.