Menu
▾
▴
Re: [Exist-development] Read lock problem in WebDAV causes database deadlock (Potentially Serious)
|
From: Patrick B. <pat...@jo...> - 2011-08-08 15:30:24
|
On Mon, Aug 8, 2011 at 12:33 AM, Adam Retter <ad...@ex...> wrote:
> I have just studied the code, and if I understand it correctly then
> there are a couple of things that seem incorrect here. It early and I
> cant sleep.
>
> Dannes is the master of all things WebDAV, so he may be able to show
> us what we are misunderstanding.
>
> Your comment about the read-lock -- it seems to me that the finally
> resource lock release, only happens if isNullResource is set, yet when
> isNullResource is set, the resource is never read locked anyway as far
> as I can tell.
>
> IMHO, this function could be improved by being split into two, its too
> long and it takes two different actions, in the least it could call
> out to a function to handle the null resource path. This would make
> the code easier to understand and test.
>
> In its current state I think line 277 should be deleted, and also line
> 294, and this should fix the locking issue.
>
This was my first thought as well (and may very well be the correct one).
But I thought there might be a reason the resource is being unlocked before
the transct.commit(). But, I haven't spent any time in the transaction
system, so I really have no idea.
>
> But in addition it seems to me, that this function reports to WebDAV
> that resources have been locked, but this is not the case as it always
> unlocks the resource. I am not sure why this would be, but I would of
> imagined that the resource should remain locked until WebDAV unlocks
> it?
>
>
I think this is actually working correctly. The function puts a user lock
and a lockToken on the resource, which (from my understanding) is the
correct way to put a persistent (user level) lock on a document. What it's
releasing is the database READ_LOCK, which can block other processes that
try to read the resource.
>
>
> On 8 August 2011 03:22, Patrick Bosek <pat...@jo...> wrote:
> > 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
> >
> >
> >
> ------------------------------------------------------------------------------
> > BlackBerry® DevCon Americas, Oct. 18-20, San Francisco, CA
> > The must-attend event for mobile developers. Connect with experts.
> > Get tools for creating Super Apps. See the latest technologies.
> > Sessions, hands-on labs, demos & much more. Register early & save!
> > http://p.sf.net/sfu/rim-blackberry-1
> > _______________________________________________
> > Exist-development mailing list
> > Exi...@li...
> > https://lists.sourceforge.net/lists/listinfo/exist-development
> >
> >
>
>
>
> --
> Adam Retter
>
> eXist Developer
> { United Kingdom }
> ad...@ex...
> irc://irc.freenode.net/existdb
>
--
Patrick Bosek
Jorsek Software
Cell (585) 820 9634
Office (877) 492 2960
Jorsek.com
|
