Closed Bug 70295 Opened 19 years ago Closed 19 years ago

PR_LockFile()/PR_UnlockFile() deadlock

Categories

(NSPR :: NSPR, defect, P3)

x86
Windows NT
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: wtc, Assigned: wtc)

Details

Attachments

(2 files)

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.
Attached patch Proposed patch.Splinter Review
I checked in the patch on the tip and NSPRPUB_RELEASE_4_1_BRANCH
of NSPR.
Status: NEW → RESOLVED
Closed: 19 years ago
Priority: -- → P3
Resolution: --- → FIXED
Target Milestone: --- → 4.1.1
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
The new fix has been checked in on the trunk and NSPRPUB_RELEASE_4_1_BRANCH.
Status: REOPENED → RESOLVED
Closed: 19 years ago19 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.