From 15de919683b1f438dc9746e82c1840c4f20666eb Mon Sep 17 00:00:00 2001 From: waterWang Date: Thu, 20 Aug 2026 13:49:16 +0800 Subject: [PATCH] fix: release START_TXN nesting level on SQLException paths in GenericDaoBase sibling methods update(ID,...), update(ub,sc,rows), expunge(ID), insertElementCollection, expunge(), unremove(ID) and remove(ID) call txn.start() which pushes a START_TXN nesting level onto the caller's transaction stack, but their SQLException catch blocks throw without reaching txn.commit() and without a finally rollback. The leaked nesting level makes the caller's commit() silently no-op, discarding all work done in the transaction. Add a committed flag and roll back in finally when commit was not reached, mirroring the fix for #13905 in persist(). Signed-off-by: waterWang --- .../com/cloud/utils/db/GenericDaoBase.java | 112 ++++++++++++------ 1 file changed, 79 insertions(+), 33 deletions(-) diff --git a/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java b/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java index dcd863465d1b..95bd71f2b094 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java +++ b/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java @@ -820,28 +820,37 @@ protected int update(ID id, UpdateBuilder ub, T entity) { } SearchCriteria sc = createSearchCriteria(); sc.addAnd(_idAttributes.get(_table)[0], SearchCriteria.Op.EQ, id); - TransactionLegacy txn = TransactionLegacy.currentTxn(); - txn.start(); - + final TransactionLegacy txn = TransactionLegacy.currentTxn(); + boolean committed = false; try { - if (ub.getCollectionChanges() != null) { - insertElementCollection(entity, _idAttributes.get(_table)[0], id, ub.getCollectionChanges()); + txn.start(); + + try { + if (ub.getCollectionChanges() != null) { + insertElementCollection(entity, _idAttributes.get(_table)[0], id, ub.getCollectionChanges()); + } + } catch (SQLException e) { + throw new CloudRuntimeException("Unable to persist element collection", e); } - } catch (SQLException e) { - throw new CloudRuntimeException("Unable to persist element collection", e); - } - int rowsUpdated = update(ub, sc, null); + int rowsUpdated = update(ub, sc, null); - txn.commit(); + txn.commit(); + committed = true; - return rowsUpdated; + return rowsUpdated; + } finally { + if (!committed) { + txn.rollback(); + } + } } public int update(UpdateBuilder ub, final SearchCriteria sc, Integer rows) { StringBuilder sql = null; PreparedStatement pstmt = null; final TransactionLegacy txn = TransactionLegacy.currentTxn(); + boolean committed = false; try { final String searchClause = sc.getWhereClause(); @@ -872,12 +881,17 @@ public int update(UpdateBuilder ub, final SearchCriteria sc, Integer rows) { int result = pstmt.executeUpdate(); txn.commit(); + committed = true; ub.clear(); return result; } catch (final SQLException e) { logger.error("DB Exception on: " + pstmt, e); handleEntityExistsException(e); throw new CloudRuntimeException("Unable to update on DB, due to: " + e.getLocalizedMessage()); + } finally { + if (!committed) { + txn.rollback(); + } } } @@ -1266,6 +1280,7 @@ public boolean expunge(final ID id) { final TransactionLegacy txn = TransactionLegacy.currentTxn(); PreparedStatement pstmt = null; String sql = null; + boolean committed = false; try { txn.start(); for (final Pair deletSql : _deleteSqls) { @@ -1281,6 +1296,7 @@ public boolean expunge(final ID id) { } txn.commit(); + committed = true; if (_cache != null) { _cache.remove(id); } @@ -1288,6 +1304,10 @@ public boolean expunge(final ID id) { } catch (final SQLException e) { logger.error("DB Exception on: " + pstmt, e); throw new CloudRuntimeException("Unable to expunge on DB, due to: " + e.getLocalizedMessage()); + } finally { + if (!committed) { + txn.rollback(); + } } } @@ -1688,34 +1708,42 @@ public T persist(final T entity) { protected void insertElementCollection(T entity, Attribute idAttribute, ID id, Map ecAttributes) throws SQLException { TransactionLegacy txn = TransactionLegacy.currentTxn(); - txn.start(); - for (Map.Entry entry : ecAttributes.entrySet()) { - Attribute attr = entry.getKey(); - Object obj = entry.getValue(); - - EcInfo ec = (EcInfo)attr.attache; - Enumeration en = null; - if (ec.rawClass == null) { - en = Collections.enumeration(Arrays.asList((Object[])obj)); - } else { - en = Collections.enumeration((Collection)obj); - } - PreparedStatement pstmt = txn.prepareAutoCloseStatement(ec.clearSql); - prepareAttribute(1, pstmt, idAttribute, id); - pstmt.executeUpdate(); + boolean committed = false; + try { + txn.start(); + for (Map.Entry entry : ecAttributes.entrySet()) { + Attribute attr = entry.getKey(); + Object obj = entry.getValue(); - while (en.hasMoreElements()) { - pstmt = txn.prepareAutoCloseStatement(ec.insertSql); - if (ec.targetClass == Date.class) { - pstmt.setString(1, DateUtil.getDateDisplayString(s_gmtTimeZone, (Date)en.nextElement())); + EcInfo ec = (EcInfo)attr.attache; + Enumeration en = null; + if (ec.rawClass == null) { + en = Collections.enumeration(Arrays.asList((Object[])obj)); } else { - pstmt.setObject(1, en.nextElement()); + en = Collections.enumeration((Collection)obj); } - prepareAttribute(2, pstmt, idAttribute, id); + PreparedStatement pstmt = txn.prepareAutoCloseStatement(ec.clearSql); + prepareAttribute(1, pstmt, idAttribute, id); pstmt.executeUpdate(); + + while (en.hasMoreElements()) { + pstmt = txn.prepareAutoCloseStatement(ec.insertSql); + if (ec.targetClass == Date.class) { + pstmt.setString(1, DateUtil.getDateDisplayString(s_gmtTimeZone, (Date)en.nextElement())); + } else { + pstmt.setObject(1, en.nextElement()); + } + prepareAttribute(2, pstmt, idAttribute, id); + pstmt.executeUpdate(); + } + } + txn.commit(); + committed = true; + } finally { + if (!committed) { + txn.rollback(); } } - txn.commit(); } @DB() @@ -2018,15 +2046,21 @@ public void expunge() { sql.append(_table).append(" WHERE ").append(_removed.first()).append(" IS NOT NULL"); final TransactionLegacy txn = TransactionLegacy.currentTxn(); PreparedStatement pstmt = null; + boolean committed = false; try { txn.start(); pstmt = txn.prepareAutoCloseStatement(sql.toString()); pstmt.executeUpdate(); txn.commit(); + committed = true; } catch (final SQLException e) { logger.error("DB Exception on: " + pstmt, e); throw new CloudRuntimeException("Unable to expunge on DB, due to: " + e.getLocalizedMessage()); + } finally { + if (!committed) { + txn.rollback(); + } } } @@ -2038,6 +2072,7 @@ public boolean unremove(ID id) { final TransactionLegacy txn = TransactionLegacy.currentTxn(); PreparedStatement pstmt = null; + boolean committed = false; try { txn.start(); pstmt = txn.prepareAutoCloseStatement(_removeSql.first()); @@ -2049,6 +2084,7 @@ public boolean unremove(ID id) { final int result = pstmt.executeUpdate(); txn.commit(); + committed = true; if (_cache != null) { _cache.remove(id); } @@ -2056,6 +2092,10 @@ public boolean unremove(ID id) { } catch (final SQLException e) { logger.error("DB Exception on: " + pstmt, e); throw new CloudRuntimeException("Unable to unremove on DB, due to: " + e.getLocalizedMessage()); + } finally { + if (!committed) { + txn.rollback(); + } } } @@ -2087,6 +2127,7 @@ public boolean remove(final ID id) { final TransactionLegacy txn = TransactionLegacy.currentTxn(); PreparedStatement pstmt = null; + boolean committed = false; try { txn.start(); @@ -2099,6 +2140,7 @@ public boolean remove(final ID id) { final int result = pstmt.executeUpdate(); txn.commit(); + committed = true; if (_cache != null) { _cache.remove(id); } @@ -2106,6 +2148,10 @@ public boolean remove(final ID id) { } catch (final SQLException e) { logger.error("DB Exception on: " + pstmt, e); throw new CloudRuntimeException("Unable to remove on DB, due to: " + e.getLocalizedMessage()); + } finally { + if (!committed) { + txn.rollback(); + } } }