#3 LockManager can lose lockinfo

[Bjorn Tyla]

conditions:
- there is 1 lock for resource 'A' and

scenario:
- acquiring thread gets lockinfo for resource 'A'
- enters the while / wait loop and blocks
- releasing thread gets lockinfo for resource 'A'
- decreases the count to 0, notifies blocking threads
and removes the lockinfo from the map
- acquirig thread wakes up (lockinfo.count == 0) and
increases the count.

result:
the acquiring thread increased the count of a lockinfo
which is no longer registered in the map thus allowing for
another thread to acquire a (exclusive) lock.

[Bjorn Tyla]

Logged In: YES
user_id=918764

Problem can be avoided by using a 'pending lockers' count in
the lockinfo which must be 0 to allow lockinfo removal.

[Tom Bradford]

Logged In: YES
user_id=35410

Have you tested this and been capable of reproducing it?
The code is setup such that the the release method
decrements the count and then calls info.notify(), which
will trigger a waiting lock acquirer to obtain a mutex on
the info object and bump up the count before returning to
the release method which then checks to see if the count is
0. The count will only be zero if there were no waiting
threads on that info object.

[Bjorn Tyla]

Logged In: YES
user_id=918764

I stumbled on this bug by reviewing the code. The scenario I
described is not entirely accurate. It is very hard to
reproducable because it depends on a thread context switch
at the right spot.

If the context switch occurs when the release and acquire
thread are both between the sync blocks ('this' and 'info'
sync) and context switches from acquire to release thus the
release reaches the info sync first then the lockinfo will be
removed because it reaches zero before the acquirer can
increase the count.

BTW on multiprocessor machines a context switch is not even
required for this bug.

BTW is it allowed to acquire multiple exclusive locks on an id?
better check the acquireExclusive while loop.

[Tom Bradford]

Logged In: YES
user_id=35410

You do realize that when you call this.notify() or
info.notify() it synchronously gives up the mutex to another
thread and waits for the scheduler to hand back the mutex...
the two do not run in parallel. Additionally there is a
second synchronized (this ) inside of the synchronized
(info) block that is used to delete the info block from the
cache.

And yes, acquireExclusive will loop and block until it can
acquire a full share of locks... It's even quite possible
that other threads can sneak in and grab a read in the mean
time, and it's even possible that two threads can deadlock
fighting for an exclusive lock on a single resource.

There's a reason why LockManager is only used in FSFiler and
not in the native implementation, partly because it's very
old (a vestige from dbXML 1.0), partly because I was never
satisfied with it, and partly because I don't know of anyone
who even uses FSFiler.

I may modify the release version of FSFiler just to do
synchronized blocks on the files themselves and not support
multiple readers, especially since the server will cache,
which should minimize the need for a multi-reader locking
system.

[Bjorn Tyla]

Logged In: YES
user_id=918764

Hi bradford,

I didn't realize that this is a deprecated class and is to be
used under controlled circumstances.

But I do believe that you are mistaken in how the the wait -
notify mechanism works.

<Java Language Specification 2 edition, chapter 17.14>
The notify method should be called for an object only when
the current thread has already locked the object's lock. If the
wait set for the object is not empty, then some arbitrarily
chosen thread is removed from the wait set and re-enabled
for thread scheduling. (Of course, that thread will not be able
to proceed until the current thread relinquishes the object's
lock.)
<end>

as for exclusive locking try this:

public class LockManagerTest {
public static void main(String[] args) {
LockManager lockMan = new LockManager(100);
lockMan.acquireExclusiveLock(0);
lockMan.acquireExclusiveLock(0);
}
}

the program finishes but I would expect it to block on the
second call.

[Tom Bradford]

Logged In: YES
user_id=35410

Ok... I've trimmed down that class and I have also
officially deprecated it. The trimmed down version uses
straight up exclusive boolean locks and will not go into a
wait state of the thread requesting the lock is the same
thread that currently has the lock. Either way, it's a
class that will probably go away unless FSFiler starts being
used.