Closed Bug 601291 Opened 14 years ago Closed 14 years ago

PR_GetCurrentThread() fails inside thread private destructor

Categories

(NSPR :: NSPR, defect, P2)

All
Linux
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: hp, Assigned: wtc)

Details

Attachments

(2 files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.3) Gecko/20100423 Ubuntu/10.04 (lucid) Firefox/3.6.3
Build Identifier: 4.8.4

PR_GetCurrentThread() uses pthread_setspecific() directly to store the thread. 
_PR_DestroyThreadPrivate() is run out of a destructor on a separate pthread_setspecific().

It doesn't seem to be reliable that the current thread stored at &current_thread_key is still there when _PR_DestroyThreadPrivate() runs. I don't know exactly what POSIX guarantees here but whatever glibc does doesn't seem to work out.

One implication of this is that you can't touch SpiderMonkey in a thread private destructor, because all its assertions that context->thread == PR_GetCurrentThread() always fail (NSPR just makes up a new PRThread* in the destructor, I think).


Reproducible: Always

Steps to Reproduce:
In the attached test program, it fails if you create a thread with pthread directly and then use PR_GetCurrentThread() in it, but it works if you create the thread using NSPR.

Actual Results:  
Destructor has wrong current PRThread.

Expected Results:  
Destructor has correct PRThread.
Attached file Test program
Attached patch Proposed patchSplinter Review
Havoc: thanks for the bug report and the test program.

POSIX Threads specifies that the thread-specific data is removed from the key before calling the thread-specific data destructor (the data is passed to the destructor as the argument):
http://www.opengroup.org/onlinepubs/000095399/functions/pthread_key_create.html

  An optional destructor function may be associated
  with each key value. At thread exit, if a key value
  has a non-NULL destructor pointer, and the thread
  has a non-NULL value associated with that key, the
  value of the key is set to NULL, and then the
  function pointed to is called with the previously
  associated value as its sole argument.

So we can try this patch.  I hope setting the thread-specific data in the destructor for it like this won't break any pthread implementation.
Attachment #480698 - Flags: review?(ted.mielczarek)
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Priority: -- → P2
Hardware: x86 → All
Target Milestone: --- → 4.8.7
Attachment #480317 - Attachment mime type: text/x-c → text/plain
Comment on attachment 480698 [details] [diff] [review]
Proposed patch

Sorry it took me a while to get to reviewing this. From the OpenGroup documentation, it sounds like this is a valid thing to do.
Attachment #480698 - Flags: review?(ted.mielczarek) → review+
Patch checked in on the NSPR trunk (NSPR 4.8.7).

Checking in ptthread.c;
/cvsroot/mozilla/nsprpub/pr/src/pthreads/ptthread.c,v  <--  ptthread.c
new revision: 3.90; previous revision: 3.89
done
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
OS: Linux → All
Resolution: --- → FIXED
OS: All → Linux
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: