1753315 - Deadlock due to reentry of module list read lock

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P3)

Description

[John Schanck [:jschanck]]

The module list lock serves two distinct purposes which, IMO, encourage reentry on the read lock:

 * This lock protects the global module lists.
 * it also protects changes to the slot array (module->slots[]) and slot count
 * (module->slotCount) in each module.

This is a problem because the module list lock is implemented as an NSSRW_Lock, which supports recursive write locking but not recursive read locking. Deadlock occurs if
1. thread A takes a read lock,
2. thread B queues to write, and then
3. thread A tries to take another read lock.
In this scenario, thread A will wait for the write lock to be released, even though the write lock is not held and thread A is itself preventing the write lock from being acquired.

Some Firefox shutdown hangs can be attributed to this pattern. In these cases, nsNSSComponent::HasActiveSmartCards takes a module list read lock in order to iterate over the module list (purpose 1). In each iteration it calls SECMOD_HasRemovableSlots which takes another read lock (pupose 2).

Ideally we would have separate locks for the global module list and the per-module slot lists, but this would involve changing the SECMODModule struct.

Alternatively we can make it the caller's responsibility to acquire the module list read lock for functions like SECMOD_HasRemovableSlots. I'll attach a patch which does this, but I'm open to suggestions.

Comment 1

[John Schanck [:jschanck]]

Attachment: Bug 1753315 - Add SECMOD_LockedModuleHasRemovableSlots. r=#nss-reviewers

Comment 2

[Robert Relyea]

So this is a public function, Other callers may already depend on SECMOD_HasRemovableSlots getting the lock itself (in fact one would expect that is what they would have done normally).

We should create a new function that doesn't hold the lock and have Firefox call the non-locking version holding the lock.

bob

Comment 3

[John Schanck [:jschanck]]

https://hg.mozilla.org/projects/nss/rev/499ae15c18adc391db748d1669d3e083b07d3dfa