505889 - PR_SetTraceOption PRTraceUnLockHandles seems to do the wrong thing

Status: RESOLVED FIXED

(NSPR :: NSPR, defect)

Description

[timeless]

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?

Comment 1

[Wan-Teh Chang]

timeless: yes, you're right.

Comment 2

[timeless]

Attachment: unlock...

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

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 4

[Wan-Teh Chang]

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

r=wtc.  Thanks!

Comment 5

[Wan-Teh Chang]

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