[Tarantool-patches] [PATCH 4/4] box/txn: fix nil dereference in txn_rollback
Cyrill Gorcunov
gorcunov at gmail.com
Mon Feb 17 22:05:08 MSK 2020
On Mon, Feb 17, 2020 at 08:19:40PM +0300, Konstantin Osipov wrote:
>
> (remove jounral_entry_complete() from here).
>
> I think once you remove journal_entry_complete (which basically
> calls rollback), you will be able to remove this ugly
> "cleanr-then-restore" of txn as well:
IOW, like below? Grigory pointed about my previous attempt. Here is
my call-gparh view (please be ready that i'm wrong)
1) Good path without error (with the patch below applied)
txn_write
txn_write_to_wal
journal_write
fiber_set_txn(fiber(), NULL);
now it is nill and txn_entry_complete_cb process just fine
txn_entry_complete_cb
assert(in_txn() == NULL);
fiber_set_txn(fiber(), txn);
txn_complete(txn);
fiber_set_txn(fiber(), NULL);
2) Error paths
a) error during journal_entry_new allocation
txn_write
txn_write_to_wal
req = journal_entry_new = NULL;
return -1;
txn_rollback (with fiber.txn != NULL)
return -1
fiber.txn is not null, so txn_rollback reverts
and set it to null, looks fine
b) error during journal_write
txn_write
txn_write_to_wal
journal_write
wal_write
...
entry->res = -1;
return -1;
txn_rollback(with fiber.txn != NULL), which
is fine.
Guys, do I miss something obvious? The key moment
is dropping journal_entry_complete call from wal.c
on error path and defer setting fiber.txn = NULL
until journal_write passed without errors.
---
Subject: [PATCH] prelim
Signed-off-by: Cyrill Gorcunov <gorcunov at gmail.com>
---
src/box/txn.c | 15 ++++++++++-----
src/box/wal.c | 1 -
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/src/box/txn.c b/src/box/txn.c
index a4ca48224..a75f8dc0b 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -495,10 +495,8 @@ txn_write_to_wal(struct txn *txn)
&txn->region,
txn_entry_complete_cb,
txn);
- if (req == NULL) {
- txn_rollback(txn);
+ if (req == NULL)
return -1;
- }
struct txn_stmt *stmt;
struct xrow_header **remote_row = req->rows;
@@ -525,6 +523,8 @@ txn_write_to_wal(struct txn *txn)
diag_log();
return -1;
}
+
+ fiber_set_txn(fiber(), NULL);
return 0;
}
@@ -583,8 +583,13 @@ txn_write(struct txn *txn)
fiber_set_txn(fiber(), NULL);
return 0;
}
- fiber_set_txn(fiber(), NULL);
- return txn_write_to_wal(txn);
+
+ if (txn_write_to_wal(txn) < 0) {
+ txn_rollback(txn);
+ return -1;
+ }
+
+ return 0;
}
int
diff --git a/src/box/wal.c b/src/box/wal.c
index 0ae66ff32..a8bab4f34 100644
--- a/src/box/wal.c
+++ b/src/box/wal.c
@@ -1209,7 +1209,6 @@ wal_write(struct journal *journal, struct journal_entry *entry)
fail:
entry->res = -1;
- journal_entry_complete(entry);
return -1;
}
--
2.20.1
More information about the Tarantool-patches
mailing list