Re: [Exist-development] BUG FIX in RpcConnection

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

Sure, will do

On Wed, Nov 2, 2011 at 6:09 PM, Adam Retter <ad...@ex...> wrote:

> Looks good to me, please by all means commit... perhaps in both 1.4.x and
> trunk?
>
> On 1 November 2011 22:39, Patrick Bosek <pat...@jo...> wrote:
> > Hi everyone,
> >
> > I've found, and fixed, a bug in the xmlrpc.RpcConnection.java. I was
> going
> > to simply commit it, but I thought it might be best to run it past the
> > developers first since I'm only 99% sure of what I've done.
> >
> > The remove(..) function on like 2615 catches any Throwable on line 2643,
> but
> > it doesn't abort it's transaction. So, if there is an exception (like the
> > target file being locked), the transaction never gets committed and never
> > gets aborted, which is bad (obviously).
> >
> > What I did to fix this was remove the aborts that sat above the
> exceptions
> > the function threw it's self and added one into the catch(Throwable)
> >
> > See here:
> >
> >
> >     private boolean remove(XmldbURI docUri) throws EXistException,
> > PermissionDeniedException {
> >         TransactionManager transact =
> > factory.getBrokerPool().getTransactionManager();
> >         Txn transaction = transact.beginTransaction();
> >         DBBroker broker = null;
> >         Collection collection = null;
> >         try {
> >             broker = factory.getBrokerPool().get(user);
> >             collection =
> broker.openCollection(docUri.removeLastSegment(),
> > Lock.WRITE_LOCK);
> >             if (collection == null) {
> > --------- START REMOVED -------------
> > //                transact.abort(transaction);
> > --------- START REMOVED -------------
> >                 throw new EXistException("Collection " +
> > docUri.removeLastSegment() + " not found");
> >             }
> >             // keep the write lock in the transaction
> >             transaction.registerLock(collection.getLock(),
> Lock.WRITE_LOCK);
> >
> >             DocumentImpl doc = collection.getDocument(broker,
> > docUri.lastSegment());
> >             if (doc == null) {
> > --------- START REMOVED -------------
> > //                transact.abort(transaction);
> > --------- END REMOVED -------------
> >                 throw new EXistException("Document " + docUri + " not
> > found");
> >             }
> >
> >             if(doc.getResourceType() == DocumentImpl.BINARY_FILE)
> >                 collection.removeBinaryResource(transaction, broker,
> doc);
> >             else
> >                 collection.removeXMLResource(transaction, broker,
> > docUri.lastSegment());
> >             transact.commit(transaction);
> >             return true;
> >
> >         } catch (Throwable e) {
> > --------- START ADDED -------------
> >             transact.abort(transaction);
> > --------- END ADDED -------------
> >             handleException(e);
> >             return false;
> >
> >         } finally {
> >             factory.getBrokerPool().release(broker);
> >         }
> >     }
> >
> >
> >
> > My only question about this is what happens in the case there is an
> > exception thrown from the transaction.commit(..)? From looking at the
> > function it wasn't immediately obvious to me if this was going to create
> an
> > issue.
> >
> >
> > Thoughts?
> >
> >
> > --
> > Patrick Bosek
> > Jorsek Software
> > Cell (585) 820 9634
> > Office (877) 492 2960
> > Jorsek.com
> >
> >
> >
> ------------------------------------------------------------------------------
> > RSA&#174; Conference 2012
> > Save $700 by Nov 18
> > Register now&#33;
> > http://p.sf.net/sfu/rsa-sfdev2dev1
> > _______________________________________________
> > 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