commit 3572f69f7dd402430d0e1725462f66529100bd9d
parent de900eefa7019be37b0fab1f421c41a078d77345
Author: Christian Grothoff <christian@grothoff.org>
Date: Fri, 7 Aug 2026 00:13:48 +0200
same commit-patch as elsewhere
Diffstat:
2 files changed, 86 insertions(+), 3 deletions(-)
diff --git a/src/donaudb/commit.c b/src/donaudb/commit.c
@@ -45,9 +45,43 @@ DONAUDB_commit (struct DONAUDB_PostgresContext *ctx)
"commit",
"COMMIT");
- qs = GNUNET_PQ_eval_prepared_non_select (ctx->conn,
- "commit",
- params);
+ {
+ PGresult *result;
+
+ result = GNUNET_PQ_exec_prepared (ctx->conn,
+ "commit",
+ params);
+ qs = GNUNET_PQ_eval_result (ctx->conn,
+ "commit",
+ result);
+ /* PostgreSQL accepts COMMIT on a transaction it has already aborted: it
+ silently rolls the transaction back and answers PGRES_COMMAND_OK with the
+ command tag ROLLBACK. GNUNET_PQ_eval_result() cannot tell that apart from
+ a real commit, so every caller took the success path for a transaction
+ whose writes are gone. Report it as a soft error instead: sessions run
+ SERIALIZABLE, so the overwhelmingly likely cause is an unhandled
+ 40001/40P01 for which retrying is right; a caller with a retry loop gives
+ up after MAX_RETRIES and fails visibly, one without fails immediately.
+
+ The *success* path deliberately keeps returning exactly what
+ GNUNET_PQ_eval_prepared_non_select() returned before. That function is
+ GNUNET_PQ_exec_prepared() + GNUNET_PQ_eval_result() plus a
+ strtol(PQcmdTuples()) step that only runs when the status is already
+ GNUNET_DB_STATUS_SUCCESS_NO_RESULTS, and PQcmdTuples() on a COMMIT command
+ tag is the empty string -- so that step yields 0, which is
+ GNUNET_DB_STATUS_SUCCESS_NO_RESULTS again. */
+ if ( (0 <= qs) &&
+ (NULL != result) &&
+ (0 == strcmp ("ROLLBACK",
+ PQcmdStatus (result))) )
+ {
+ GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
+ "Transaction `%s' was rolled back by the database instead of committed; a previous statement must have failed without being handled\n",
+ ctx->transaction_name);
+ qs = GNUNET_DB_STATUS_SOFT_ERROR;
+ }
+ PQclear (result);
+ }
ctx->transaction_name = NULL;
return qs;
}
diff --git a/src/donaudb/test_donaudb.c b/src/donaudb/test_donaudb.c
@@ -22,6 +22,7 @@
#include <taler/taler_json_lib.h>
#include "donaudb_lib.h"
#include "donau_util.h"
+#include "helper.h"
#include "donaudb_lib.h"
#include "donau-database/commit.h"
#include "donau-database/create_tables.h"
@@ -425,6 +426,54 @@ run (void *cls)
DONAUDB_preflight (ctx);
+ /* A transaction PostgreSQL has already aborted must never be reported as
+ committed -- and a clean transaction must still return exactly
+ GNUNET_DB_STATUS_SUCCESS_NO_RESULTS, which is what every caller that
+ compares against 0 relies on. */
+ {
+ struct GNUNET_PQ_ExecuteStatement mk[] = {
+ GNUNET_PQ_make_execute ("CREATE TABLE test_commit_rollback (x INT)"),
+ GNUNET_PQ_EXECUTE_STATEMENT_END
+ };
+ struct GNUNET_PQ_ExecuteStatement bad[] = {
+ GNUNET_PQ_make_execute ("SELECT 1/0"),
+ GNUNET_PQ_EXECUTE_STATEMENT_END
+ };
+ struct GNUNET_PQ_ExecuteStatement chk[] = {
+ GNUNET_PQ_make_execute (
+ "DO $$ BEGIN"
+ " IF EXISTS (SELECT FROM pg_class"
+ " WHERE relname='test_commit_rollback')"
+ " THEN RAISE EXCEPTION 'table survived a rollback';"
+ " END IF; END $$"),
+ GNUNET_PQ_EXECUTE_STATEMENT_END
+ };
+
+ FAILIF (GNUNET_OK !=
+ DONAUDB_start (ctx,
+ "test-commit-aborted"));
+ FAILIF (GNUNET_OK !=
+ GNUNET_PQ_exec_statements (ctx->conn,
+ mk));
+ /* provoke an error; from here on the transaction is doomed */
+ FAILIF (GNUNET_OK ==
+ GNUNET_PQ_exec_statements (ctx->conn,
+ bad));
+ /* before the fix this returned GNUNET_DB_STATUS_SUCCESS_NO_RESULTS (0) */
+ FAILIF (0 <=
+ DONAUDB_commit (ctx));
+ /* ...and the table is indeed gone, so 'success' would have been a lie */
+ FAILIF (GNUNET_OK !=
+ GNUNET_PQ_exec_statements (ctx->conn,
+ chk));
+
+ FAILIF (GNUNET_OK !=
+ DONAUDB_start (ctx,
+ "test-commit-clean"));
+ FAILIF (GNUNET_DB_STATUS_SUCCESS_NO_RESULTS !=
+ DONAUDB_commit (ctx));
+ }
+
result = 0;
drop:
