On 26/07/08 08:41AM, Patrick Steinhardt wrote:

On Tue, Jul 07, 2026 at 11:14:08PM -0500, Justin Tobler wrote:

quoted

diff --git a/odb/transaction.c b/odb/transaction.c
index df4275151b..51af2c9a61 100644
--- a/odb/transaction.c
+++ b/odb/transaction.c

@@ -16,19 +16,26 @@ int odb_transaction_begin(struct object_database *odb,
 	return ret;
 }
 
-void odb_transaction_commit(struct odb_transaction *transaction)
+int odb_transaction_commit(struct odb_transaction *transaction)
 {
+	int ret;
+
 	if (!transaction)
-		return;
+		return 0;
 
 	/*
 	 * Ensure the transaction ending matches the pending transaction.
 	 */
 	ASSERT(transaction == transaction->source->odb->transaction);
 
-	transaction->commit(transaction);
+	ret = transaction->commit(transaction);
+	if (ret)
+		return ret;
+
 	transaction->source->odb->transaction = NULL;
 	free(transaction);
+
+	return 0;
 }

Doesn't this cause a leak now?

Good call. Ya, if odb_transaction_commit() fails, we don't free the
transaction. In the next version I'll go ahead and clear the transaction
if we fail.

I think this interface here is doing the same mistake that our reference
transactions did, where we automatically released the transaction on
commit. That caused multiple lifetime issues with references all over
the place.

Ya, it probaby makes sense to introduce a separate
`odb_transaction_release()` function to make this explicit and update
callers accordingly. I have another series I working on that introduces
`odb_transaction_abort()`. This might be a good place to add it in too.

-Justin