Skip to content

v0.2.0 — post-review hardening

Choose a tag to compare

View all tags
@EvgeniyPatlan EvgeniyPatlan released this 13 May 19:46
· 73 commits to main since this release
v0.2.0
de83179

TidesDB MySQL plugin — v0.2.0

First release after two full code-review rounds. 50 findings closed across CRITICAL / HIGH / MEDIUM / LOW severity, plus the previously-undocumented "ENGINE_ATTRIBUTE freeze" constraint. All 37/37 hand-rolled and 58/58 MTR tests pass (2 intentional skips unchanged).

Pull

docker pull evgeniypatlan/test-images:mysql-9.7-tidesdb-v0.2.0
# or
docker pull evgeniypatlan/test-images:mysql-9.7-tidesdb-latest

Image digest: sha256:5001022248f83b923b24f971aabd0e8b145f42c549315b5596d841728b8de516
Size: 967 MB
Base: Oracle Linux 9 + gcc-toolset-14 (Stage 1) → mysql:9.7 (Stage 2)

Quick smoke

docker run --rm -d --name tidesdb-v020 -p 3307:3306 \
  -e MYSQL_ROOT_PASSWORD=root \
  evgeniypatlan/test-images:mysql-9.7-tidesdb-v0.2.0

# wait ~10 seconds for mysqld to come up
docker exec -it tidesdb-v020 mysql -uroot -proot -e \
  "SELECT engine, support, transactions FROM information_schema.engines WHERE engine='TidesDB';"
# expect: TidesDB | YES | YES

What changed

The full review reports landed alongside the fixes (docs/code-review-report.md, docs/code-review-followup-report.md). Headline items below.

CRITICAL (3 fixed)
C-1 Encryption silently emitted ciphertext under uninitialized stack key/IV when encryption_key_get or my_random_bytes failed. Now checks both return values, secure-zeroes the stack key on every exit path via tdb_secure_zero (and now explicit_bzero where available).
C-2 tdb_global was a plain pointer cleared on shutdown without synchronization vs concurrent handler threads — null-deref crash on shutdown-under-load. Now std::atomic<tidesdb_t *> accessed via tdb_get_engine() / tdb_set_engine(); shutdown uses exchange(nullptr, acq_rel).
CF-1 The H-3 trx-lifecycle rwlock rollout missed tidesdb_hton_kill_query — a UAF if KILL QUERY raced a connection close. Now takes the read-lock around the trx deref.

HIGH (13 fixed)

  • H-1 deadlock detector now re-runs on each cond_wait wakeup (was indefinite hang under specific contention patterns).
  • H-2 / HF-2 FTS doc/word counter RMW serialized — now per-share (was process-global mutex serializing unrelated tables).
  • H-3 deadlock walker derefs are protected by a global mysql_rwlock_t (g_trx_lifecycle_lock); close_connection takes the write lock around my_free(trx).
  • H-4 Hilbert spatial encoder operator-precedence bug — (uint32_t)s << 1 was truncating before shift; widened hilbert_rot's n to uint64_t. Roughly half of all spatial keys were encoded wrong pre-fix.
  • H-5 tidesdb_s3_secret_key + tidesdb_s3_access_key hidden via PLUGIN_VAR_NOSYSVAR (no longer visible via SHOW VARIABLES to anyone with SYSTEM_VARIABLES_ADMIN).
  • H-6 / HF-3 path-traversal defense on tidesdb_backup_dir / tidesdb_checkpoint_dir: rejects relative paths and .. components; new tidesdb_backup_allowed_root sysvar for full confinement.
  • H-9 ENGINE_ATTRIBUTE JSON parser switched to kParseIterativeFlag + 64KB length cap — no stack overflow / heap exhaustion via deeply nested JSON.
  • H-10 ICP was advertised but the check stub always returned 0 — secondary-index range scans leaked rows past end_range. Now actually evaluates pushed_idx_cond->val_bool() + compare_key_icp + thd_killed. Pre-fix MTR baselines for tidesdb_sql / tidesdb_stress were encoding the bug.
  • HF-1 M-12 stopword-table privilege check now fail-closed on NULL THD.
  • HF-4 backup/checkpoint refuse immediately if THD already killed (uncancellable from MySQL side once started).

MEDIUM (21 fixed)

  • ENGINE_ATTRIBUTE options cached on the share with atomic publish (was per-call JSON re-parse + 25 THDVAR reads).
  • FTS doc/word delta buffered during bulk INSERT, flushed once at end_bulk_insert (was 1 RMW per row).
  • TLS-cached master key with generation-counter invalidation — no mutex on the hot decrypt path.
  • Master-key page mlock + MADV_DONTDUMP; smart 1-page-vs-2-page based on actual straddle.
  • Per-handler scratch buffers via std::unique_ptr thread-local pointer (destructor runs at thread exit; no pooled-thread leak).
  • getrandom(2) for IVs (was open/read/close /dev/urandom per row).
  • Log-injection defense — tdb_sanitize_for_log on user-supplied strings.
  • Stopword loader's table scan moved outside the write-lock.
  • key_unpack_scratch_ sized at open() rather than per-seek.
  • ... plus a dozen more, see docs/code-review-report.md.

LOW (13 fixed)

S3 endpoint/bucket redacted in error log; explicit_bzero where available; lock-order documentation; null-byte path check; end_bulk_insert propagates flush rc; mlock errno mapped to operator hints; tests renamed from finding-ID to feature-based.

ENGINE_ATTRIBUTE-freeze constraint closed

ALTER TABLE t ENGINE_ATTRIBUTE='{"...":...}' previously appeared to succeed (DD updated, SHOW CREATE TABLE reflected the new value) but the engine kept using the pre-alter parse for the share's lifetime. Now commit_inplace_alter_table computes fresh opts and atomically swaps the share's cached pointer, plus refreshes share->default_ttl / share->isolation_level / share->encrypted. New regression test: tidesdb_alter_engine_attribute.

What's deferred (and why)

Three findings remain open by design, with code comments documenting the rationale:

  • M-6 Field::pack virtual-dispatch fast path — codec hot path; correctness risk too high without benchmarks proving the speedup.
  • M-10 Lock-entry recycling — bound to the H-3 UAF invariant that lock entries are never freed. Recycling needs extending the rwlock to cover lock-entry lifetime. Memory growth bounded per distinct PK ever locked.
  • LF-5 Per-row atomic acquire-load of g_master_key_gen — single mov on x86 with no fence; eliminating it would require API change to plumb a handler-level cache into the free function. Disproportionate effort for negligible gain.

Architectural work outstanding (not bugs, not blocked)

Per the refreshed architect's priority list at the end of docs/code-review-followup-report.md:

  1. Extract EngineContext + promote tidesdb_keyring_compat.cc to a real tidesdb_crypto/ module.
  2. Extract row-lock manager to tidesdb_row_lock.{h,cc} with an opaque RowLockManager.
  3. Extract FTS to tidesdb_fts.{h,cc} (4 duplicated thread_local scratch sites collapse into one FtsIndex).
  4. TidesStore abstraction over the TidesDB C API.
  5. MySQL 9.7 atomic-DDL participation (SDI callbacks).
  6. Delete #if 0 dead corners and MariaDB-only methods.
  7. Explicit state machine for inplace ALTER.
  8. Audit / shrink tidesdb_compat.h.

Breaking changes

None. v0.2.0 is a drop-in replacement for v0.1.0.

Acknowledgements

Two thorough multi-lens code reviews (C++ correctness / security / performance / architecture) ran across plugin/ and surfaced every finding in this release. The reports themselves (docs/code-review-report.md, docs/code-review-followup-report.md) are committed alongside the code so the audit trail is preserved.

Assets 2
Loading