Hello,
I tried to build all modules under xwiki-platform-core, but the build
process hanged on PreemtiveLockTest, so I had a look at the code and
noticed some problems:
1. PreemtiveLockProvider.lockBlockingThread is concurrently accessed and
updated despite being an unsynchronized HashMap. It must be synchronized.
2. The deadlock recovery algorithm is "if there is a deadlock, ignore
the fact that the lock is locked and proceed." To me this seems
unintuitive. When a deadlock is detected, an exception should be thrown
and the thread aborted.
3. Presumably, the thread which is deprived of its lock is supposed to
at least be blocked until the thieving thread unlocks the lock, but
there is a race condition so both threads may run simultaneously.
4. Related to 3., the 'owners' stack in the lock is concurrently
accessed without synchronization in 'currentThreadOwnsAllLocks'.
As of deadlock detection, do we really need it? If so, would
java.lang.management.ThreadMXBean.findDeadlockedThreads() be sufficient?
In my opinion, the implementation PreemtiveLock should be discarded and
the LockProvider should return an ordinary ReentrantReadWriteLock.
Best Regards,
/Andreas