bsddragonfly-bsd

DragonflyBSD: possible race-condition in lock manager (kern_lock.c) code?


Lately I've been reading the lock_manager (kern_lock.c) code, and bumped into some scenario which I think would create a race-condition.

Step 1:

@undo_shreq(...): if there is an upgrade request pending, the code will reset "LKC_UPREQ" flag and do the wakeup() call; this thing happen only if

(count & (LKC_EXREQ | LKC_UPREQ | LKC_CANCEL)) &&

       (count & (LKC_SMASK | LKC_XMASK)) == 0)

Step 2:

Now, in-parallel, an another thread T2 is trying to get an exclusive lock and it reaches the trivial condition (i.e) @lockmgr_exclusive(...)

if ((count & (LKC_UPREQ | LKC_EXREQ |
      LKC_XMASK)) == 0 &&
    ((count & LKC_SHARED) == 0 ||
     (count & LKC_SMASK) == 0))

So, T2 increases the count by 1 and set itself as the owner thread-- meaning it got the exclusiveness.

Step 3:

A thread (T1) slept on a LKC_UPREQ flag woke by Step 1; and here is the code after sleep (....after, LK_SLEEPFAIL and sleep-error sanity check), @lockmgr_upgrade(...)

if ((count & LKC_UPREQ) == 0) {           // reset by step 1
KKASSERT((count & LKC_XMASK) == 1);      // true, by step 2
lkp->lk_lockholder = td;
break;
}

I see (please correct me if am wrong), at step 3 that, thread T1 resets the lk_lockholder to itself-- meaning, it got the exclusiveness!


Solution

  • The way it works is that if one thread sets UPREQ and then sleeps, another thread will grant it the exclusive lock and wake it up. The granting thread clears UPREQ and increments the exclusive count, but does not know 'who' set the UPREQ so it is then up to the thread that set UPREQ to set the lockholder field.

    Since the lockmgr code must deal with both many-exclusive-to-single-shared starvation, many-shared-to-single-exclusive starvation cases, AND deadlock edge cases, it is fairly complex. Edge cases have been found over the last year or two, but this particular case doesn't look like a bug to me. Just a bit confusing because the exclusive lock is granted (UPREQ cleared and excl count incremented) by the second thread but the first thread is still responsible for setting the lk_lockholder field.