ovsdb-idl: Plug hole in state machine.

The state machine didn't have a proper state for "not yet committed or
aborted", which meant that destroying an ovsdb_idl_txn without committing
or aborting it caused a segfault.  This fixes the problem by adding a new
state TXN_UNCOMMITTED to the state machine.

This is related to commit 79554078d "ovsdb-idl: Fix bad logic in
ovsdb_idl_txn_commit() state transitions", which fixed a related bug.

Bug #2438.

diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index f4daaed..51a62dd 100644 (file)
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -1127,6 +1127,8 @@ const char *
 ovsdb_idl_txn_status_to_string(enum ovsdb_idl_txn_status status)
 {
     switch (status) {
+    case TXN_UNCOMMITTED:
+        return "uncommitted";
     case TXN_UNCHANGED:
         return "unchanged";
     case TXN_INCOMPLETE:
@@ -1153,7 +1155,7 @@ ovsdb_idl_txn_create(struct ovsdb_idl *idl)
     txn->request_id = NULL;
     txn->idl = idl;
     hmap_init(&txn->txn_rows);
-    txn->status = TXN_INCOMPLETE;
+    txn->status = TXN_UNCOMMITTED;
     txn->error = NULL;
     txn->dry_run = false;
     ds_init(&txn->comment);
@@ -1226,7 +1228,7 @@ ovsdb_idl_txn_destroy(struct ovsdb_idl_txn *txn)
 void
 ovsdb_idl_txn_wait(const struct ovsdb_idl_txn *txn)
 {
-    if (txn->status != TXN_INCOMPLETE) {
+    if (txn->status != TXN_UNCOMMITTED && txn->status != TXN_INCOMPLETE) {
         poll_immediate_wake();
     }
 }
@@ -1532,6 +1534,7 @@ ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn)
                        "transact", operations, &txn->request_id))) {
         hmap_insert(&txn->idl->outstanding_txns, &txn->hmap_node,
                     json_hash(txn->request_id, 0));
+        txn->status = TXN_INCOMPLETE;
     } else {
         txn->status = TXN_TRY_AGAIN;
     }
@@ -1569,7 +1572,7 @@ void
 ovsdb_idl_txn_abort(struct ovsdb_idl_txn *txn)
 {
     ovsdb_idl_txn_disassemble(txn);
-    if (txn->status == TXN_INCOMPLETE) {
+    if (txn->status == TXN_UNCOMMITTED || txn->status == TXN_INCOMPLETE) {
         txn->status = TXN_ABORTED;
     }
 }

diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
index d11fb0e..ac61e1a 100644 (file)
--- a/lib/ovsdb-idl.h
+++ b/lib/ovsdb-idl.h
@@ -111,6 +111,7 @@ bool ovsdb_idl_row_is_synthetic(const struct ovsdb_idl_row *);
 /* Transactions. */
 
 enum ovsdb_idl_txn_status {
+    TXN_UNCOMMITTED,            /* Not yet committed or aborted. */
     TXN_UNCHANGED,              /* Transaction didn't include any changes. */
     TXN_INCOMPLETE,             /* Commit in progress, please wait. */
     TXN_ABORTED,                /* ovsdb_idl_txn_abort() called. */

diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index 5956f72..f9c8286 100644 (file)
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -221,6 +221,34 @@ OVSDB_CHECK_IDL([simple idl, increment operation],
 003: done
 ]])
 
+OVSDB_CHECK_IDL([simple idl, aborting],
+  [['["idltest",
+      {"op": "insert",
+       "table": "simple",
+       "row": {}}]']],
+  [['set 0 r 2.0, abort' \
+'+set 0 b 1']],
+  [[000: i=0 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1>
+001: commit, status=aborted
+002: commit, status=success
+003: i=0 r=0 b=true s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1>
+004: done
+]])
+
+OVSDB_CHECK_IDL([simple idl, destroy without commit or abort],
+  [['["idltest",
+      {"op": "insert",
+       "table": "simple",
+       "row": {}}]']],
+  [['set 0 r 2.0, destroy' \
+'+set 0 b 1']],
+  [[000: i=0 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1>
+001: destroy
+002: commit, status=success
+003: i=0 r=0 b=true s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1>
+004: done
+]])
+
 OVSDB_CHECK_IDL([self-linking idl, consistent ops],
   [],
   [['["idltest",

diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
index 5594b40..e8d87b6 100644 (file)
--- a/tests/test-ovsdb.c
+++ b/tests/test-ovsdb.c
@@ -1828,6 +1828,13 @@ idl_set(struct ovsdb_idl *idl, char *commands, int step)
             }
             ovsdb_idl_txn_increment(txn, arg1, arg2, NULL);
             increment = true;
+        } else if (!strcmp(name, "abort")) {
+            ovsdb_idl_txn_abort(txn);
+            break;
+        } else if (!strcmp(name, "destroy")) {
+            printf("%03d: destroy\n", step);
+            ovsdb_idl_txn_destroy(txn);
+            return;
         } else {
             ovs_fatal(0, "unknown command %s", name);
         }

diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index df20f06..c6fc8a4 100644 (file)
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -3642,6 +3642,7 @@ do_vsctl(const char *args, struct vsctl_command *commands, size_t n_commands,
     txn = the_idl_txn = NULL;
 
     switch (status) {
+    case TXN_UNCOMMITTED:
     case TXN_INCOMPLETE:
         NOT_REACHED();