Re: [Exist-development] BUG FIX in RpcConnection

[Dmitriy S. <sha...@gm...>]

For me this safe solution & fix. Transactions ALWAYS MUST be in
try-catch-final block.

BTW, transaction API can be improved by success()/fail() + finish() methods.

tx = db.startTransaction();
try {
    ....
    if (....) {
        throw new ...
     }
    ....
    tx.success();
} catch {
    tx.fail();
} finally {
    tx.finish();
}

On Wed, Nov 2, 2011 at 2:39 AM, 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
>
>


-- 
Dmitriy Shabanov