Details
-
Type:
Bug
-
Status:
Open
-
Priority:
Critical
-
Resolution: Unresolved
-
Affects Version/s: None
-
Fix Version/s: None
-
Component/s: Storage
-
Labels:None
-
Difficulty:Trivial
-
Similar issues:
XWIKI-1736Transaction partially committed when database encoding differs from the java encoding XWIKI-5365Rolling back a document doesn't roll back its correct attachments version XWIKI-5364Rolling back document to previous version doesn't bring back deleted attachment XWIKI-5474Do not save documents in the cache when they are being saved to the database. XWIKI-4297 add XWiki.saveDocuments for saving multiple documents in a single db transaction. XWIKI-342 XWiki.renameDocument fails silently on failed object copy XWIKI-3009Document with an image link is not parsed back correctly XWIKI-7045Property names in GroupsClass, LevelsClass and UsersClass are not set when new properties are created, which may cause upgrade failure. XWIKI-5001WYSIWYG may loose or change style attribute, depending on browser used XWIKI-3826When hitting "Cancel' in inline mode and the document doesn't exist yet, the user should be sent back to the previous page
Description
This question: http://xwiki.markmail.org/message/tdht5ecsnjswmqg3 still haven't got a satisfying answer. I'm not a database expert, but I agree with Sergiu that the code is wrong.
An example of when this occur is when a document with an attachment is moved to the recycle bin, then you use a script to create a new document with the same name and with an attachment with the same file name. The save will fail without rolling back the transaction. (Whether the save should fail or not in this situation is a different question, but clearly, the database should remain consistent.)
The method endTransaction should look something like this:
/**
* Ends a transaction
*
* @param context
* @param commit should we commit or not
* @param withTransaction
*/
protected void endTransaction(XWikiContext context, boolean commit, boolean withTransaction)
{
Session session = getSession(context);
Transaction transaction = getTransaction(context);
if (transaction != null) {
try {
setSession(null, context);
setTransaction(null, context);
// We need to clean up our connection map first because the connection will
// be aggressively closed by hibernate 3.1 and more
preCloseSession(session);
if (log.isDebugEnabled()) {
log.debug("Releasing hibernate transaction " + transaction);
}
if (commit) {
transaction.commit();
} else {
transaction.rollback();
}
} catch (Exception e) {
try {
transaction.rollback();
} catch (Exception e) {
/*
* The rollback failed. Thus, the database might
* be left in an inconsistent state. So, maybe we
* should throw a java.io.IOError here? I have
* not seen any example code on how to deal with
* this situation, and I don't see how we can
* recover from this situation.
*/
}
throw new RuntimeException(e);
} finally {
closeSession(session);
}
}
}
One problem raised off line is that the database must also support transactions. So, even if we fix the code, on MySQL with the default MyISAM storage engine the database could still end up in a bad state, since a transaction rollback is meaningless.