Menu
▾
▴
Re: [Exist-development] Read lock problem in WebDAV causes database deadlock (Potentially Serious)
|
From: Adam R. <ad...@ex...> - 2011-08-10 09:08:15
|
>> 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.
I think that lockToken is only used for WebDAV and nothing else unless
I am mistaken?
> What it's
> releasing is the database READ_LOCK, which can block other processes that
> try to read the resource.
READ_LOCK does not block other reads, it only blocks other writes.
>>
>> 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
>
>
--
Adam Retter
eXist Developer
{ United Kingdom }
ad...@ex...
irc://irc.freenode.net/existdb
|
