diff --git a/cc/ss2pl/transaction.cc b/cc/ss2pl/transaction.cc index 8936b2f1..b1437957 100644 --- a/cc/ss2pl/transaction.cc +++ b/cc/ss2pl/transaction.cc @@ -158,11 +158,9 @@ Status TxExecutor::read(Storage s, std::string_view key, TupleBody** body) { if (tuple == nullptr) return Status::WARN_NOT_FOUND; read_internal(s, key, tuple); -#if defined(DLR0) if (this->status_ == TransactionStatus::aborted) { return Status::ERROR_LOCK_FAILED; } -#endif *body = &(read_set_.back().body_); FINISH_READ: @@ -342,11 +340,9 @@ Status TxExecutor::update(Storage s, std::string_view key, TupleBody&& body) { #if ADD_ANALYSIS result_->local_write_latency_ += rdtscp() - start; #endif // ADD_ANALYSIS -#if defined(DLR0) if (this->status_ == TransactionStatus::aborted) { return Status::ERROR_LOCK_FAILED; } -#endif return Status::OK; } diff --git a/docs/coding-conventions_en.md b/docs/coding-conventions_en.md index 61ca8701..d6378ab6 100644 --- a/docs/coding-conventions_en.md +++ b/docs/coding-conventions_en.md @@ -69,7 +69,10 @@ Starts thin. Add as you learn. ### `tx.read` returns Status — *check it* `tx.read` returns `Status::OK` on hit and `Status::WARN_NOT_FOUND` on miss. -On miss, `*body` is **left unchanged** (it does not get nullified). Checking +Some protocols return other errors as well (`Status::ERROR_LOCK_FAILED` for +SS2PL and MOCC, `Status::ERROR_PREEMPTIVE_ABORT` for TicToc). **Reject every +non-`Status::OK` value** — do not test for one specific value. On any non-OK +result, `*body` is **left unchanged** (it does not get nullified). Checking only `tx.status_ == TransactionStatus::aborted` is not enough — a stale `body` pointer from a previous read will silently get dereferenced, which manifests as a `HeapObject::cast_to` assertion under Debug+ASan and as diff --git a/docs/coding-conventions_ja.md b/docs/coding-conventions_ja.md index 9ed3190e..5f94f5eb 100644 --- a/docs/coding-conventions_ja.md +++ b/docs/coding-conventions_ja.md @@ -62,7 +62,7 @@ ### `tx.read` の戻り値 Status は **必ず check する** -`tx.read` はヒット時に `Status::OK`、ミス時に `Status::WARN_NOT_FOUND` を返す。ミス時には `*body` は **そのまま** (= nullify されない) なので、`tx.status_ == TransactionStatus::aborted` だけチェックしても不十分 — 前回の read で得た古い `body` ポインタがそのまま deref されてしまい、Debug+ASan では `HeapObject::cast_to` の assertion で死に、Release では garbage data を読む。必ず次の形で書く: +`tx.read` はヒット時に `Status::OK`、ミス時に `Status::WARN_NOT_FOUND` を返す。protocol によっては、これ以外の error も返る (SS2PL と MOCC の `Status::ERROR_LOCK_FAILED`、TicToc の `Status::ERROR_PREEMPTIVE_ABORT` など)。**`Status::OK` 以外は全部拒否すること** — 特定の値だけを見てはいけない。`Status::OK` 以外のとき `*body` は **そのまま** (= nullify されない) なので、`tx.status_ == TransactionStatus::aborted` だけチェックしても不十分 — 前回の read で得た古い `body` ポインタがそのまま deref されてしまい、Debug+ASan では `HeapObject::cast_to` の assertion で死に、Release では garbage data を読む。必ず次の形で書く: ```cpp Status stat = tx.read(s, key, &body); diff --git a/include/bomb_pessimistic.hh b/include/bomb_pessimistic.hh index 9e49e006..6e8f4f64 100644 --- a/include/bomb_pessimistic.hh +++ b/include/bomb_pessimistic.hh @@ -695,8 +695,10 @@ public: for (auto& m_id : query.args.i_id_set) { SimpleKey<8> key; MaterialCostMaster::CreateKey(query.args.f_id, m_id, key.ptr()); - TupleBody* body; - tx.read(Storage::MaterialCostMaster, key.view(), &body); + TupleBody* body = nullptr; + Status stat = tx.read(Storage::MaterialCostMaster, key.view(), &body); + if (tx.status_ == TransactionStatus::aborted) return; + if (stat != Status::OK) return; MaterialCostMaster& old = body->get_value().cast_to(); HeapObject obj; obj.template allocate(); diff --git a/include/tpcc/tpcc_tx_delivery.hh b/include/tpcc/tpcc_tx_delivery.hh index 28ed6e98..bee855b6 100644 --- a/include/tpcc/tpcc_tx_delivery.hh +++ b/include/tpcc/tpcc_tx_delivery.hh @@ -149,8 +149,8 @@ bool update_order_line_and_get_ol_total(TxExecutor& tx, uint16_t w_id, status = tx.update(Storage::OrderLine, ol_key, TupleBody(ol_key, std::move(ol_obj))); - if (status != Status::OK) ERR; if (tx.status_ == TransactionStatus::aborted) { return false; } + if (status != Status::OK) ERR; ol_total += ol.OL_AMOUNT; } return true;