Mercurial > urweb
changeset 2000:bddee3af70c4
Tweaking uw_commit() logic, partly to fix a resource clean-up bug on SQL serialization failures
author | Adam Chlipala <adam@chlipala.net> |
---|---|
date | Tue, 15 Apr 2014 19:12:49 -0400 (2014-04-15) |
parents | a1d3fbdcc897 |
children | 16f5f136a807 |
files | doc/manual.tex src/c/urweb.c |
diffstat | 2 files changed, 44 insertions(+), 18 deletions(-) [+] |
line wrap: on
line diff
--- a/doc/manual.tex Mon Feb 24 09:10:31 2014 +0000 +++ b/doc/manual.tex Tue Apr 15 19:12:49 2014 -0400 @@ -2464,7 +2464,7 @@ \end{verbatim} All side effects in Ur/Web programs need to be compatible with transactions, such that any set of actions can be undone at any time. Thus, you should not perform actions with non-local side effects directly; instead, register handlers to be called when the current transaction is committed or rolled back. The arguments here give an arbitary piece of data to be passed to callbacks, a function to call on commit, a function to call on rollback, and a function to call afterward in either case to clean up any allocated resources. A rollback handler may be called after the associated commit handler has already been called, if some later part of the commit process fails. A free handler is told whether the runtime system expects to retry the current page request after rollback finishes. - Any of the callbacks may be \texttt{NULL}. To accommodate some stubbornly non-transactional real-world actions like sending an e-mail message, Ur/Web treats \texttt{NULL} \texttt{rollback} callbacks specially. When a transaction commits, all \texttt{commit} actions that have non-\texttt{NULL} rollback actions are tried before any \texttt{commit} actions that have \texttt{NULL} rollback actions. Thus, if a single execution uses only one non-transactional action, and if that action never fails partway through its execution while still causing an observable side effect, then Ur/Web can maintain the transactional abstraction. + Any of the callbacks may be \texttt{NULL}. To accommodate some stubbornly non-transactional real-world actions like sending an e-mail message, Ur/Web treats \texttt{NULL} \texttt{rollback} callbacks specially. When a transaction commits, all \texttt{commit} actions that have non-\texttt{NULL} rollback actions are tried before any \texttt{commit} actions that have \texttt{NULL} rollback actions. Furthermore, an SQL \texttt{COMMIT} is also attempted in between the two phases, so the nicely transactional actions have a chance to influence whether data are committed to the database, while \texttt{NULL}-rollback actions only get run in the first place after committing data. The reason for all this is that it is \emph{expected} that concurrency interactions will cause database commits to fail in benign ways that call for transaction restart. A truly non-undoable action should only be run after we are sure the database transaction will commit. When a request handler ends with multiple pending transactional actions, their handlers are run in a first-in-last-out stack-like order, wherever the order would otherwise be ambiguous.
--- a/src/c/urweb.c Mon Feb 24 09:10:31 2014 +0000 +++ b/src/c/urweb.c Tue Apr 15 19:12:49 2014 -0400 @@ -3304,32 +3304,58 @@ } } + if (ctx->transaction_started) { + int code = ctx->app->db_commit(ctx); + + if (code) { + if (ctx->client) + release_client(ctx->client); + + if (code == -1) { + // This case is for a serialization failure, which is not really an "error." + // The transaction will restart, so we should rollback any transactionals + // that triggered above. + + for (i = ctx->used_transactionals-1; i >= 0; --i) + if (ctx->transactionals[i].rollback != NULL) + ctx->transactionals[i].rollback(ctx->transactionals[i].data); + + for (i = ctx->used_transactionals-1; i >= 0; --i) + if (ctx->transactionals[i].free) + ctx->transactionals[i].free(ctx->transactionals[i].data, 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 = ctx->used_transactionals-1; i >= 0; --i) if (ctx->transactionals[i].rollback == NULL) if (ctx->transactionals[i].commit) { ctx->transactionals[i].commit(ctx->transactionals[i].data); if (uw_has_error(ctx)) { - uw_rollback(ctx, 0); + if (ctx->client) + release_client(ctx->client); + + for (i = ctx->used_transactionals-1; i >= 0; --i) + if (ctx->transactionals[i].rollback != NULL) + ctx->transactionals[i].rollback(ctx->transactionals[i].data); + + for (i = ctx->used_transactionals-1; i >= 0; --i) + if (ctx->transactionals[i].free) + ctx->transactionals[i].free(ctx->transactionals[i].data, 0); + return 0; } } - if (ctx->transaction_started) { - int code = ctx->app->db_commit(ctx); - - if (code) { - if (code == -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) { delta *d = &ctx->deltas[i]; client *c = find_client(d->client);