Closed Bug 505889 Opened 11 years ago Closed 11 years ago

PR_SetTraceOption PRTraceUnLockHandles seems to do the wrong thing

Categories

(NSPR :: NSPR, defect, critical)

defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: timeless, Assigned: timeless)

References

()

Details

(Keywords: coverity)

Attachments

(1 file)

510         case PRTraceLockHandles :
511             PR_LOG( lm, PR_LOG_DEBUG,
512                 ("PRSetTraceOption: PRTraceLockTraceHandles"));
513             PR_Lock( traceLock );
514             break;
515         
516         case PRTraceUnLockHandles :
517             PR_LOG( lm, PR_LOG_DEBUG,
518                 ("PRSetTraceOption: PRTraceUnLockHandles"));
519             PR_Lock( traceLock );
520             break;

shouldn't UnLockHandles ... well ... unlock them instead of locking them?
timeless: yes, you're right.
Attached patch unlock...Splinter Review
Assignee: wtc → timeless
Status: NEW → ASSIGNED
Attachment #390297 - Flags: review?
Attachment #390297 - Flags: review? → review?(wtc)
PRLocks are not re-entrant.  That is, if a thread holds a lock, and tries
to acquire it a second time, it deadlocks itself, IINM.  

Consequently, I suspect that this pair of settable options, 
PRTraceLockHandles and PRTraceUnLockHandles, are never tested and never used.
If they were, then the first time that a thread tried to use 
PRTraceUnLockHandles, after it had previously used PRTraceLockHandles, it
would hang.  If they were used, there would be bug reports about this.  

So, I wonder ... do we want to try to fix this, or decommit support for it?  
The NSPR team, such as it now is, doesn't have any spare cycles to sustain
dead functionality.  I'd rather remove it than keep it.  

Comments?
Comment on attachment 390297 [details] [diff] [review]
unlock...

r=wtc.  Thanks!
Attachment #390297 - Flags: review?(wtc) → review+
I checked in the patch on the NSPR trunk (NSPR 4.8.1).

Checking in prtrace.c;
/cvsroot/mozilla/nsprpub/pr/src/misc/prtrace.c,v  <--  prtrace.c
new revision: 3.13; previous revision: 3.12
done
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → 4.8.1
You need to log in before you can comment on or make changes to this bug.