Proper handling of serialization failures during SQL COMMIT

7 files changed, 57 insertions, 20 deletions

diff --git a/include/urweb/urweb_cpp.h b/include/urweb/urweb_cpp.h
index 8dfffdf9..248e54e4 100644
--- a/include/urweb/urweb_cpp.h
+++ b/include/urweb/urweb_cpp.h
@@ -40,7 +40,8 @@ failure_kind uw_begin(struct uw_context *, char *path);
 void uw_ensure_transaction(struct uw_context *);
 failure_kind uw_begin_onError(struct uw_context *, char *msg);
 void uw_login(struct uw_context *);
-void uw_commit(struct uw_context *);
+int uw_commit(struct uw_context *);
+// ^-- returns nonzero if the transaction should be restarted
 int uw_rollback(struct uw_context *, int will_retry);
 
 __attribute__((noreturn)) void uw_error(struct uw_context *, failure_kind, const char *fmt, ...);
diff --git a/src/c/cgi.c b/src/c/cgi.c
index c9ec744a..f1482589 100644
--- a/src/c/cgi.c
+++ b/src/c/cgi.c
@@ -134,9 +134,10 @@ void uw_copy_client_data(void *dst, void *src) {
 }
 
 void uw_do_expunge(uw_context ctx, uw_Basis_client cli, void *data) {
-  uw_ensure_transaction(ctx);
-  uw_get_app(ctx)->expunger(ctx, cli);
-  uw_commit(ctx);
+  do {
+    uw_ensure_transaction(ctx);
+    uw_get_app(ctx)->expunger(ctx, cli);
+  } while (uw_commit(ctx) && (uw_rollback(ctx, 1), 1));
 }
 
 void uw_post_expunge(uw_context ctx, void *data) {
diff --git a/src/c/fastcgi.c b/src/c/fastcgi.c
index d6d2391d..bbda0f57 100644
--- a/src/c/fastcgi.c
+++ b/src/c/fastcgi.c
@@ -632,9 +632,10 @@ void uw_copy_client_data(void *dst, void *src) {
 }
 
 void uw_do_expunge(uw_context ctx, uw_Basis_client cli, void *data) {
-  uw_ensure_transaction(ctx);
-  uw_get_app(ctx)->expunger(ctx, cli);
-  uw_commit(ctx);
+  do {
+    uw_ensure_transaction(ctx);
+    uw_get_app(ctx)->expunger(ctx, cli);
+  } while (uw_commit(ctx) && (uw_rollback(ctx, 1), 1));
 }
 
 void uw_post_expunge(uw_context ctx, void *data) {
diff --git a/src/c/http.c b/src/c/http.c
index c57740e9..9050aaf4 100644
--- a/src/c/http.c
+++ b/src/c/http.c
@@ -447,9 +447,10 @@ void uw_copy_client_data(void *dst, void *src) {
 }
 
 void uw_do_expunge(uw_context ctx, uw_Basis_client cli, void *data) {
-  uw_ensure_transaction(ctx);
-  uw_get_app(ctx)->expunger(ctx, cli);
-  uw_commit(ctx);
+  do {
+    uw_ensure_transaction(ctx);
+    uw_get_app(ctx)->expunger(ctx, cli);
+  } while (uw_commit(ctx) && (uw_rollback(ctx, 1), 1));
 }
 
 void uw_post_expunge(uw_context ctx, void *data) {
diff --git a/src/c/request.c b/src/c/request.c
index 5973d979..b925cc3c 100644
--- a/src/c/request.c
+++ b/src/c/request.c
@@ -116,8 +116,10 @@ static void *periodic_loop(void *data) {
           return NULL;
     } while (r == UNLIMITED_RETRY || (r == BOUNDED_RETRY && retries_left > 0));
 
-    if (r != FATAL && r != BOUNDED_RETRY)
-      uw_commit(ctx);
+    if (r != FATAL && r != BOUNDED_RETRY) {
+      if (uw_commit(ctx))
+	r = UNLIMITED_RETRY;
+    }
 
     sleep(p->pdic.period);
   };
diff --git a/src/c/urweb.c b/src/c/urweb.c
index 3082f110..57f57694 100644
--- a/src/c/urweb.c
+++ b/src/c/urweb.c
@@ -3253,13 +3253,13 @@ static char *find_sig(char *haystack) {
   return s;
 }
 
-void uw_commit(uw_context ctx) {
+int uw_commit(uw_context ctx) {
   int i;
   char *sig;
 
   if (uw_has_error(ctx)) {
     uw_rollback(ctx, 0);
-    return;
+    return 0;
   }
 
   for (i = ctx->used_transactionals-1; i >= 0; --i)
@@ -3268,7 +3268,7 @@ void uw_commit(uw_context ctx) {
         ctx->transactionals[i].commit(ctx->transactionals[i].data);
         if (uw_has_error(ctx)) {
           uw_rollback(ctx, 0);
-          return;
+          return 0;
         }
       }
 
@@ -3278,13 +3278,26 @@ void uw_commit(uw_context ctx) {
         ctx->transactionals[i].commit(ctx->transactionals[i].data);
         if (uw_has_error(ctx)) {
           uw_rollback(ctx, 0);
-          return;
+          return 0;
         }
       }
 
-  if (ctx->transaction_started && ctx->app->db_commit(ctx)) {
-    uw_set_error_message(ctx, "Error running SQL COMMIT");
-    return;
+  if (ctx->transaction_started) {
+    int code =ctx->app->db_commit(ctx);
+
+    if (code) {
+      if (code == -1) {
+	uw_rollback(ctx, 1);
+	return 1;
+      }
+
+      for (i = ctx->used_transactionals-1; i >= 0; --i)
+	if (ctx->transactionals[i].free)
+	  ctx->transactionals[i].free(ctx->transactionals[i].data, 0);
+
+      uw_set_error_message(ctx, "Error running SQL COMMIT");
+      return 0;
+    }
   }
 
   for (i = 0; i < ctx->used_deltas; ++i) {
@@ -3390,6 +3403,8 @@ void uw_commit(uw_context ctx) {
       } while (sig);
     }
   }
+
+  return 0;
 }
 
 
diff --git a/src/postgres.sml b/src/postgres.sml
index 272097e7..8cfa5f48 100644
--- a/src/postgres.sml
+++ b/src/postgres.sml
@@ -438,7 +438,23 @@ fun init {dbstring, prepared = ss, tables, views, sequences} =
          newline,
          newline,
          string "if (PQresultStatus(res) != PGRES_COMMAND_OK) {",
-         box [string "PQclear(res);",
+         box [string "if (!strcmp(PQresultErrorField(res, PG_DIAG_SQLSTATE), \"40001\")) {",
+              box [newline,
+                   string "PQclear(res);",
+                   newline,
+                   string "return -1;",
+                   newline],
+              string "}",
+              newline,
+              string "if (!strcmp(PQresultErrorField(res, PG_DIAG_SQLSTATE), \"40P01\")) {",
+              box [newline,
+                   string "PQclear(res);",
+                   newline,
+                   string "return -1;",
+                   newline],
+              string "}",
+              newline,
+	      string "PQclear(res);",
               newline,
               string "return 1;",
               newline],