Thread-unsafe access to global variable pt_debug

NEW
Unassigned

Status

2 years ago
2 years ago

People

(Reporter: jld, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

2 years ago
NSPR's thread library, if compiled with DEBUG, tries to keep some stats in the global variable pt_debug.  However, it's updated with normal thread-unsafe C operations.  In some cases lost updates to debugging stats might not be a significant problem, but this isn't one of them: the values in question are all balanced pairs of counters (created/destroyed, acquired/released, etc.), so lost updates will affect whether they balance as expected.

Normally I'd suggest using atomic operations to update the counters, but NSPR supports atomic operations on platforms that don't have them natively by polyfilling with locks, so that would be a circular dependency and probably self-deadlock.  The counters could just be disabled completely on those platforms.

Alternately, given that this has been broken since 1998[*] as far as I can tell, and nobody's noticed, maybe we could just remove the code entirely.

This bug was found by Helgrind, so I don't have a test case that demonstrates it, but starting a bunch of threads that do nothing but lock/unlock different locks should do it.


[*]: https://hg.mozilla.org/projects/nspr/diff/81fc3c4cf2b3/pr/include/private/primpl.h
You need to log in before you can comment on or make changes to this bug.