The index's staleness argument rested on "every load path that writes
location/contents/next directly calls db_validate_refs() afterwards, and
that invalidates." Checking it rather than asserting it: db_free() has
eight call sites and db_validate_refs() has one, so the argument does not
hold in general.
It happens to hold at each non-SELFCHECK site today -- db_read(),
db_make_minimal() and the SQLite open path each either run with an empty
index or have the following setters re-invalidate every container they
touch -- so this is a latent gap rather than a live defect. But it is
latent by coincidence, and the reasoning that establishes it is exactly
the kind that rots on the next refactor.
db_free() is the one point all eight paths share, and it is where the
containers the index was built from cease to exist. Invalidating there
retires the argument instead of restating it.
Also hoists the sqlite_load_game() invalidation out of the per-object
loop, where it was clearing the whole map once per object rather than
once per load.
make test: 36 targets, 34 passed, 2 skipped, 0 failed
(jit=yes stubslave=yes realitylvls=yes wodrealms=yes)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
match_list() compared the query against every object in scope and could not
stop at the first hit, because ambiguity has to be detected. Name resolution
was 66.8% of attrread and 74.8% of attrwrite once the storage costs were out
of the attribute path (#2073, #2084).
THE INDEX IS A FILTER, NOT THE DECIDER. string_compare() folds Unicode case
through mux_tolower(), whose output is not a byte-for-byte map; reimplementing
that to build a hash key would be an approximation that silently disagrees
with the walk on some name. So the bucket key is an ASCII normalisation only,
every candidate it yields is still confirmed with the real string_compare(),
and any name carrying a byte >= 0x80 is not bucketed -- it goes on a must-scan
list that suppresses the short-circuit for that container. A wrong key costs
a fallback, never a wrong answer.
Skipping the remaining walk on a hit is sound because an exact match scores
CON_COMPLETE|CON_LOCAL (0x09 minimum) while a partial tops out at
CON_LOCAL|CON_TYPE|CON_LOCK (0x07): exact always outranks partial.
Only used when md.check_keys is false. With it set, promote_match() evaluates
could_doit() per candidate -- softcode -- so skipping the walk would skip those
evaluations. Exit and movement matching, the only check_keys users, keep the
existing behaviour exactly.
INVALIDATION. s_Location/s_Contents/s_Next/s_Name carry hooks, which covers
the 45 call sites that mutate contents lists through them. Three sites bypass
the setters and write the fields directly; grepping for those found them, and
they invalidate too:
* db.cpp objref() fixup loop and the SQLite load path -- whole-index clear.
* object.cpp force_reclaim_failed_create_slot(), which turns an object into
GARBAGE while clearing its fields. That one is in normal operation and
would have left a container's index naming a destroyed object.
MEASURED. Per lookup at 1000 objects, the tax that scaled with scope is gone
-- name lookup now costs what a dbref lookup costs:
lookup master with index
by dbref (ctrl) 26.7us 30.0us
name at head 53.3us 26.7us
name at middle 60.0us 23.3us
name at tail 66.7us 26.7us
Per command, two runs, with three non-resolving controls flat:
phase before run1 run2
attrwrite 143us 46us 41us
attrread 160us 51us 55us
dispatch (ctrl) 31us 30us 32us
softcode_arith 27us 28us 26us
Behaviour verified identical to master across fifteen cases: exact match,
case-insensitivity, partial-match fallthrough, space compression, true
ambiguity (two objects sharing a name still give #-2), rename, movement and
destruction.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
collect_attrnums_from_storage() answered every atr_head/atr_next with a
GetAll() query. $-command matching reaches it once per object in scope on every
line a player types, so a master room with 200 global commands cost one SQLite
round trip per object per command -- ~67% of $-dispatch was inside sqlite3.c,
its WAL frame lookups and its fcntl locking. Attribute VALUES were cached; the
attribute LIST never was.
Cache the storage half per object. The pending overlay
(cache_collect_pending_attrnums) still runs on every lookup and is deliberately
NOT cached with it, because it changes as the write queue drains.
Invalidation is in two places and the second is not redundant:
* cache_put/cache_del, when an attribute may appear or disappear. This also
covers the bStandAlone branches, which write through to SQLite directly and
never reach a flush.
* cache_flush_writes, once the queue has landed in SQLite. Without this, a
list cached BETWEEN queue and flush stays correct only while the overlay
still reports the pending attribute; the moment the flush clears the dirty
flag the overlay goes quiet and the stale list silently loses it.
Attribute value changes do not affect the list, so they do not invalidate.
MEASURED, live server, typed commands matching nothing, master room populated
with objects carrying one $-command each. Both columns from the same build:
objects master with cache us/object master -> fix
- 120us 80us
25 270us 90us 3.0x 6.0 -> 0.4
50 430us 110us 3.9x 6.2 -> 0.6
100 750us 170us 4.4x 6.3 -> 0.9
200 1580us 290us 5.4x 7.3 -> 1.1
The empty-master-room baseline improves too (120us -> 80us): atr_head serves
far more than $-matching.
This retires the $-pattern automaton idea for now. Profiling the same workload
put wild() + wild_lit_eq() at 0.48% of user time with 200 patterns tested per
command -- getting to the patterns was the cost, not comparing them.
Correctness, beyond `make test`:
* pending ADD visible before flush.
* a list cached BEFORE a flush still reports the attribute AFTER it
(KEEP moves from position 3 to 73 as 70 pads land) -- the case the
flush-side invalidation exists for.
* tombstone suppressed immediately (#1045), and still suppressed after the
delete itself flushes, rather than resurrecting from a stale list.
`make test` -- 34 passed, 2 skipped, 0 failed
(jit=yes stubslave=yes nls=no realitylvls=yes wodrealms=yes).
Sibling of #2073, which fixed the pending-merge half of the same function.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Convert remaining engine `int x = mux_atoi64(...)` (and explicit
static_cast<int> forms) to int64_t so parse width is not discarded on
any platform. Includes functions/funceval/funmath, mail folder numbers,
comsys charge parse, JIT register indices, and HIR lower mid/left/right.
dbref parse keeps full-width parse then rejects values outside int
range. Channel charge still stores int after an in-range check.
Residual Phase 3 player/staff prose that still used T() on live paths:
connect/reconnect/disconnect room and monitor lines, [Suspect] flags,
GAME: restart/shutdown/signal/log-full, dual-path channel boot, and
@email hostname resolution. pot + xx complete + ko msgmerge append;
tools/merge_nls_markings.py for Windows agents without xgettext
(canonical pot remains make -C mux/po pot on Linux).
Replace ~900 typographic \xE2\x80\x.. escapes (curly quotes, en dashes)
and a few \xE2\x80\230 octal workarounds with real UTF-8 in message
strings under mux/modules and mux/src. Leave stringutil and convert
charset mapping tables as explicit byte sequences.
Completes the sweep the issue called for. mux_atol returns long, which
is 32-bit on LLP64, so every caller silently truncated on Windows. Two
of those were real defects (the truthiness family and cf_size, fixed in
the preceding commits); the rest were latent, waiting for a value large
enough to matter.
Rather than audit 290 sites for whether each can reach 2^31 today, use
the 64-bit parser everywhere and remove the class. A dbref cannot
overflow now, but nothing stops a later caller passing that same site a
timestamp or a byte count.
Pure 1:1 substitution: 285 lines changed, and every removed line
contained mux_atol while every added line contains mux_atoi64. No
control flow, no types, no behaviour beyond the wider parse.
This is a NO-OP on LP64 -- long is already 64-bit on Linux and macOS, so
the generated code there is unchanged. It only widens the parse on
Windows. Narrowing destinations are unaffected either way: `int x =
mux_atoi64(s)` truncates exactly as `int x = mux_atol(s)` did, on both
models.
Left alone: mux_atol itself in mathutil, its declaration, and three
comments that name it. Callers that genuinely want 32-bit semantics can
still ask for them; none appear to.
Verified on Windows: full solution builds clean with no new warnings,
smoke is 1418 passed / 16 failed / 0 crashes / 306 of 306 dispatched --
identical to before the sweep, with the same 16 build-configuration
failures (exp3 module not loaded, hmac/digest behind UNIX_DIGEST).
Spot checks after the change: the boolean family returns 1 for multiples
of 2^32, cf_size round-trips 3000000000 and still reads -1 as unlimited,
and arithmetic, string and list functions are unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Restart (#1039–#1043): zero process-local DESC fields after load,
restore peer via getpeername, strip non-survivable TLS/WS flags,
drop TLS and WebSocket before exec, tear down adopt failures with
shutdownsock, abort partial restart loads cleanly.
DB (#1044–#1047): cache_flush_writes returns success only after
Commit; leave dirty queue on failure; atr_head union SQLite with
pending cache/queue; set mudstate.bStandAlone in DbConvert; flush
attr queue before import Commit.
Queue (#1048–#1052): setup_que refunds on OOM; sql_que frees BQUE
on Query fail and inits waittime; do_wait rolls back semaphore if
wait_que fails; include nest depth limit 50; CPU halt_que uses
Owner+object for machines.
JIT (#1053–#1057): full 64-bit rv_load_i64; setq/r single-char
RegisterSet; decline long CARGS; accumulate inline u() watermarks;
bounded guest_strnlen on ECALL paths.
Conf/convert (#1058–#1061): @enable/@disable sync g_dc; live
driver knobs for output/retry/max_players/timeouts/quota; safe
heap encode for t5x/t6h/r7h attributes.
For an exit, the location field holds the destination, and @link
exit=home stores the HOME sentinel there ("HOME is always linkable",
parse_linkable_room; move.cpp handles dest==HOME throughout; decompile
emits "@link <exit>=home" for it). db_validate_refs (#810) clamped
location with the plain objref() check, so every home-linked exit was
rewritten to an unlinked exit on load -- and the repair persists on the
next dump. Give location the same HOME-permitting treatment link
already gets.
Also corrects the comment cross-references in db.cpp/engine_com.cpp,
which cited #809 (the unrelated a64 DBT issue) instead of #810.
Adds a smoke regression test (db_validate_refs.mux): an exit opened
with @open smoke_home_exit=home must still have loc() == #-3 after the
flat-export -> SQLite-import -> load round-trip. Against the previous
clamp it fails with loc=#-1; with the fix the suite passes 1116/1116.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Object field dbrefs (location/contents/exits/next/link/owner/parent/zone) were
stored straight from the DB with no range validation. Object indices (#806) and
attribute numbers (#808) were validated, but the field VALUES were not, and the
engine has no db_check. A wild value crashes at use: DOLIST (db.h:218) walks
contents/exits via Next(thing)=db[thing].next, guarding only NOTHING/self-loops,
so a wild next -> db[wild].next is an OOB read; location/parent/zone/owner index
db[] elsewhere. db_read clamped only zone; sqlite_load_game clamped nothing.
Add db_validate_refs(), called once after a load completes (engine_com.cpp
LoadGame, covering warm-SQLite and cold-flatfile paths). It clamps each object's
location/contents/exits/next/parent/zone to a real object or NOTHING, link
additionally to HOME, and a wild owner to GOD. For a valid DB every field is
already in range, so it is a no-op there; only a corrupt DB is repaired (the
repair persists on the next dump).
Verified: a server loaded from SQLite with next=999999999 on object 0 starts
cleanly and logs "db_validate_refs: clamped 1 out-of-range object reference(s)";
smoke 1115/1115 (valid DBs unaffected). Same incomplete-hardening family as
#806/#807/#808.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Loading a corrupt/malicious DB with an out-of-range user-attribute number
corrupted memory or OOMed: the number indexes anum_table via anum_set (a bare
`anum_table[x]=v` macro) and anum_extend (allocates a dense (x+1) table), with
no validation. The read path atr_num validates anum<0||>top; the write path
(vattr_define_LEN -> anum_extend/anum_set) did not (incomplete hardening,
family of #805/#806/#807).
Live-verified via flatfile import (+A record, novel attr name):
- +A-10000000 -> anum_table[-10000000] write -> SIGSEGV.
- +A-5 -> anum_table[-5] write in valid heap -> silent corruption.
- +A999999999 -> anum_extend allocs ~8GB dense table -> OOM kill.
File-derived numbers reach this from the flatfile +A handler (getref) and the
SQLite attr-name load (db.cpp:3626). A huge number also seeds attr_next enormous
via g_max_nam_atr/max_attrnum_loaded -> later runtime OOM.
Fix:
- New A_USER_MAX (0x01000000 = 16M) constant in attrs.h beside A_USER_START.
- vattr_define_LEN rejects number < A_USER_START || > A_USER_MAX (central backstop).
- The flatfile +A handler and the SQLite attr-name callback validate and skip
the bad record (graceful degradation; flatfile path logs it).
Verified bite-then-not-bite: malformed +A flatfiles SIGSEGV/silent-corrupt/
OOM-kill on the old build, load cleanly (bad record skipped) on the fixed build;
valid DB still imports; smoke 1115/1115.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Use the new LBuf_Adopt() macro to take RAII ownership of caller-owned
pool buffers returned by atr_get, atr_pget, and atr_get_LEN. This
eliminates ~82 explicit free_lbuf calls and automatically covers
early-return paths that previously required careful manual placement
of the free.
Heaviest conversions: player.cpp (13), comsys.cpp (14), command.cpp (9).
Complex interleaved patterns (did_it charge/runout swaps, PureName
reassignment, process_cmdent loops) left manual for now.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Second wave: convert manual alloc/free pairs in cque (1), db_rw (4),
conf (5), db (2), engine_com (6), help (7), levels (2), predicates (5),
player (7), funmath (8). Eliminates ~80 explicit free_lbuf calls
including multi-exit error paths in getboolexp1 (5 frees → 0),
get_list, AnnounceConnect/Disconnect, and the eight NOEVAL function
variants (cand/cor/firstof/allof and bool variants).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Story 2a: Extend IStorageBackend with Count(), GetModCount(), and
GetAllModCounts(). Implement in CSQLiteBackend. Route all attribute
access in attrcache.cpp and db.cpp through the interface, eliminating
GetDB() bypasses for attribute-specific operations.
Story 2b: New docs/attribute-metadata.md defines the per-attribute
metadata contract: (object, attrnum) -> (value, owner, flags, mod_count),
encoding requirements, mod_count semantics, and iteration guarantees.
Story 2c: Confirmed @search never queries the attributes table directly.
SQL fast-paths use object metadata only; eval= predicates go through
cache_get() -> IStorageBackend. No changes needed for backend swap.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Eliminate the ISOUTOFMEMORY macro that unconditionally aborted on allocation
failure. Each of the 23 call sites now handles OOM appropriately:
- Fatal sites (buffer pools, db array, anum table): mux_assert or OutOfMemory
- Recoverable sites (queue, mail, commands, guests, vattrs, config, restart,
forward lists): log the failure and return gracefully
Also fix g_dump_child_pid portability: volatile pid_t -> volatile sig_atomic_t
with explicit casts in ganl_adapter.cpp.
Close integer overflow issue as false alarm (getstring_noalloc uses a bounded
static buffer, so nBuffer+1 cannot wrap).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
LogStatBuf() was defined but never called. GAME_DOOFERMUX was never
defined, making all four #ifdef blocks dead code (A_REGINFO attribute,
RegInfo table entry, registration stamp on player create, and alternate
execl argv[0]). Update smoke test expectations.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Default 1000, 0 = unlimited. Wizard-owned objects are exempt.
Enforced in atr_add_raw_LEN with a fast cache_get existence check
(updates are free) and SQLite COUNT(*) for new attributes only.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add LBuf class to alloc.h: an RAII wrapper around alloc_lbuf/free_lbuf
that moves LBUF_SIZE buffers from the stack to the heap pool. Convert
all 108 non-static UTF8 xxx[LBUF_SIZE] stack arrays across 25 source
files. Static BSS buffers (24) are unchanged.
This eliminates LBUF_SIZE from recursive stack frames, making it safe
to increase LBUF_SIZE without risking stack overflow in the evaluation
pipeline.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The previous sentinel-based approach (UINT32_MAX) wrapped to 0 on
the next write, breaking monotonicity. The before-transaction
eager mutation approach diverged on rollback.
New design: two-phase collect/apply.
Phase 1 (before transaction): attr_mod_count_collect_object()
reads all attrs from SQLite and the in-memory map, returning a
vector of {key, current_value} pairs. No in-memory mutation.
Phase 2 (after successful commit only):
attr_mod_count_apply_increments() writes current_value+1 to the
in-memory map for each collected entry.
On rollback: no in-memory mutation occurred, so the map stays
consistent with SQLite. Monotonicity is preserved because the
increment is always current+1, never a sentinel or reset.
Smoke tests: 610/610 passing.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The UINT32_MAX sentinel wrapped to 0 on next write, breaking
monotonicity. Replace with:
- attr_mod_count_invalidate_object() now seeds all attr counters
from SQLite BEFORE deletion (via GetAllAttrModCounts), then
increments each by 1. Counters are strictly monotonic — no
sentinel, no wrapping, no reuse across delete/recreate cycles.
- Call sites in object.cpp moved BEFORE DelAllAttributes so the
SQLite rows are still readable when seeding.
- ClearAttributes() clears the in-memory map entirely (correct
for full database reload where JIT cache is also cleared).
Smoke tests: 610/610 passing.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two bugs from code review:
1. attr_mod_count_inc ran before cache_put/cache_del, so failed
writes bumped the in-memory counter without changing SQLite.
Fix: increment AFTER successful storage. PutAttribute reads
the current counter and passes counter+1 to the upsert
independently.
2. DelAllAttributes and ClearAttributes bypassed mod_count tracking.
Fix: attr_mod_count_invalidate_object() sets all entries for a
destroyed object to UINT32_MAX (always stale). ClearAttributes()
calls attr_mod_count_invalidate_all() to clear the entire map.
Smoke tests: 610/610 passing.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two bugs from code review:
1. In-memory counter started at 0 regardless of persisted value.
After restart, first write to an attr with mod_count=17 in SQLite
would set in-memory to 1 while SQLite advanced to 18.
Fix: attr_mod_count_inc() seeds from SQLite on first access.
2. Delete + recreate reused mod_count=1, making stale cache entries
look fresh. Fix: increment in-memory counter BEFORE deletion
(survives the SQLite row removal), and PutAttribute uses
MAX(excluded.mod_count, mod_count+1) so the SQLite value is
always >= the in-memory value.
Smoke tests: 610/610 passing.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Foundation for Tier 3 u() inlining cache staleness detection.
Each attribute now tracks a modification counter that increments
on every atr_add/atr_clr. The JIT compiler will record mod_count
at compile time and check for staleness at runtime.
Schema changes:
- Bump to v8: ALTER TABLE attributes ADD COLUMN mod_count
- PutAttribute uses INSERT...ON CONFLICT DO UPDATE to atomically
increment mod_count on each write
- New GetAttrModCount() method for SQLite lookup
In-memory tracking:
- attr_mod_count_inc() incremented from atr_add_raw_LEN / atr_clr
- attr_mod_count_get() reads from in-memory map with SQLite fallback
- Survives LRU cache eviction (separate map, not in AttrCacheEntry)
Also fixes CURRENT_SCHEMA_VERSION (was 6, now 8).
Smoke tests: 610/610 passing.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Four new features identified from the TinyMUSH/PennMUSH/RhostMUSH survey:
MSSP (MUD Server Status Protocol, telnet option 70):
- Server sends structured key-value data (NAME, PLAYERS, UPTIME, PORT,
CODEBASE, FAMILY) to MU* directory crawlers on IAC DO MSSP
- Stateless response via send_mssp() in telnet.cpp using g_dc config basket
- start_time_utc added to DRIVER_CONFIG for uptime calculation
GMCP (Generic MUD Communication Protocol, telnet option 201):
- Protocol negotiation: server offers WILL GMCP, tracks gmcp_enabled per DESC
- Inbound: GMCP subneg queued as synthetic "\x01GMCP" command, dispatched
to handle_gmcp() which fires A_GMCP attribute with %0=package %1=json
- Outbound: gmcp(<player>, <package>, <json>) softcode function sends
GMCP frames to all GMCP-enabled descriptors via SendGmcp COM method
- Full COM architecture: mux_IConnectionManager::SendGmcp in driver,
send_gmcp() bridge in engine, CConnectionManager impl in modules.cpp
@protect (player name reservation):
- @protect[/add] <name>, @protect/del <name>, @protect/list [<player>]
- A_PROTECTNAME attribute (234) stores space-separated protected names
- protectname_check() hooked into create_player() and do_name()
- max_name_protect config param (default 5)
benchmark(<expression>, <iterations>):
- FN_NOEVAL, CA_PUBLIC, 10000 iteration cap
- Uses clock_gettime(CLOCK_MONOTONIC) / QueryPerformanceCounter
- Returns elapsed seconds as floating point
Also fixes: add unicode_tables.c to libmux.so LIBMUX_C_SRC (resolves
pre-existing tr_tolower_sbt etc. link errors).
505/505 smoke tests passing.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
