[Exist-development] Read lock problem in WebDAV causes database deadlock (Potentially Serious)

[Patrick B. <pat...@jo...>]

Hi Everyone,

After many hours of investigation I've found a locking situation in the
WebDAV module which causes database deadlocks. The problem (as I see it) is
that the webdav.methods.Lock.process() sets a READ_LOCK and has several
cases where it exits without releasing it:


-- webdav.methods.Lock.process() -- line 201:

resource = broker.getXMLResource(path,
org.exist.storage.lock.Lock.READ_LOCK);

....

-- webdav.methods.Lock.process() -- line 249:* !!!!! As you can see in the
case where the resource is already locked, it exists without releasing it's
READ_LOCK*

                // Check if Resource is already locked.
                if( lock!=null && !lock.getName().equals(user.getName()) ){
                    LOG.debug("Resource is locked.");
                    response.sendError(SC_RESOURCE_IS_LOCKED,
                            "Resource is locked by user "+ user.getName()
+".");
                    return;
                }

                // Check for request fo shared lock.
                if(lockToken.getScope() == LockToken.LOCK_SCOPE_SHARED) {
                    LOG.debug("Shared locks are not implemented.");

response.sendError(HttpServletResponse.SC_NOT_IMPLEMENTED,
                            "Shared locks are not implemented.");
                    return;
                }

                // Fill new Locktoken with new UUID
                lockToken.createOpaqueLockToken();
                resource.getMetadata().setLockToken(lockToken);
                resource.setUserLock(user);

                // Make token persistant
                TransactionManager transact = pool.getTransactionManager();
                Txn transaction = transact.beginTransaction();
                broker.storeXMLResource(transaction, resource);
                //TOUNDERSTAND : this lock is released below (in the finally
clause)
                //isn't it rather a user lock release attempt ?
                // ?

resource.getUpdateLock().release(org.exist.storage.lock.Lock.READ_LOCK); *!!!!!
Here is where it is released when no one else has it locked*
                transact.commit(transaction);



So, this is an issue which occurs in multi-user systems when two users try
to access the same document.

My proposed fix is to simply add releases to the two return cases. I don't
really understand why the resource isn't just being unlocked in the finally
in all cases, but I'm guessing there is a reason to release it before
committing the transaction that I don't understand.

My Fix:

                if( lock!=null && !lock.getName().equals(user.getName()) ){
                    LOG.debug("Resource is locked.");
                    response.sendError(SC_RESOURCE_IS_LOCKED,
                            "Resource is locked by user "+ user.getName()
+".");


resource.getUpdateLock().release(org.exist.storage.lock.Lock.READ_LOCK);
                    return;
                }

                // Check for request fo shared lock.
                if(lockToken.getScope() == LockToken.LOCK_SCOPE_SHARED) {
                    LOG.debug("Shared locks are not implemented.");

response.sendError(HttpServletResponse.SC_NOT_IMPLEMENTED,
                            "Shared locks are not implemented.");


resource.getUpdateLock().release(org.exist.storage.lock.Lock.READ_LOCK);
                    return;
                }


I can provide a patch or simply commit this into the 1.4.x branch if
requested to do so by one of the core developers. But I wanted to ask first,
since there are things going on here that are above my head.

Thoughts?


Cheers,

-- 
Patrick Bosek
Jorsek Software
Cell (585) 820 9634
Office (877) 492 2960
Jorsek.com