70295 - PR_LockFile()/PR_UnlockFile() deadlock

Status: RESOLVED FIXED

(NSPR :: NSPR, defect, P3)

Description

[Wan-Teh Chang]

This bug was reported by Chris Elving.

In PR_LockFile(), the lock _pr_flock_lock is held around calls to
_PR_MD_LOCKFILE().  This same lock brackets calls to
_PR_MD_UNLOCKFILE() in PR_UnlockFile().

This leads to a possible deadlock situation.

For example, the following scenario with two threads, T1 and T2, in
the same process operating on one file:

1. T1 calls PR_LockFile(fd1)
      i. T1 acquires _pr_flock_lock without blocking
      ii. T1 fcntl()/LockFileEx()s without blocking
      iii. T1 releases _pr_flock_lock
2. T2 calls PR_LockFile(fd2)
      i. T2 acquires _pr_flock_lock without blocking
      ii. T2 blocks on fcntl()/LockFileEx() as T1 holds a lock
3. T1 calls PR_UnlockFile(fd1)
      i. T1 blocks on _pr_flock_lock as T2 holds a lock

T2 is blocked on step 2. ii. and T1 is blocked on step 3. i.

This deadlock is easily reproducible on Windows NT.

Comment 1

[Wan-Teh Chang]

Attachment: Proposed patch.

Comment 2

[Wan-Teh Chang]

I checked in the patch on the tip and NSPRPUB_RELEASE_4_1_BRANCH
of NSPR.

Comment 3

[Wan-Teh Chang]

My previous fix has a problem.  It leaves fd->secret->lockCount
with the value -1 if _PR_MD_LOCKFILE() fails.  Then the other
threads get stuck in the PR_WaitCondVar() call waiting for
lockCount to change.

I have a new fix that sets fd->secret->lockCount to 0 if
_PR_MD_LOCKFILE() fails.  This new patch has been reviewed by
larryh and celving.  I am going to attach it to this bug report
next.

Comment 4

[Wan-Teh Chang]

Attachment: The new patch that sets fd->secret->lockCount to 0 if _PR_MD_LOCKFILE() fails.

Comment 5

[Wan-Teh Chang]

The new fix has been checked in on the trunk and NSPRPUB_RELEASE_4_1_BRANCH.