diff --git a/AGENTS.md b/AGENTS.md index 57d5ef9..6c13e00 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -19,6 +19,7 @@ R CMD check --no-manual --compact-vignettes=gs+qpdf mori_*.tar.gz # matches CI ``` - `Config/build/compilation-database: true` in DESCRIPTION makes R generate `compile_commands.json` on install, so clangd C navigation works out of the box (gitignored). +- R's in-source build does not track header dependencies: after editing `mori.h`, `rm src/*.o src/*.so` before reinstalling — otherwise stale objects compiled against the old struct layouts get linked in (sizeof mismatches, garbage fields). ## Storage Model @@ -28,7 +29,7 @@ R CMD check --no-manual --compact-vignettes=gs+qpdf mori_*.tar.gz # matches CI ## Concurrency Model -mori is **write-once on host, read-many on consumers, COW for any mutation.** Each `share()` allocates a new SHM region with a unique name; existing regions are never mutated. The name embeds the creator PID (`mori__`); the counter starts at a per-process random value (seeded on first use in `mori_shm_name`), so colliding with a dead same-PID incarnation's names additionally requires a ~2^-32 counter overlap; `mori_shm_create` opens with `O_EXCL` (POSIX) / checks `ERROR_ALREADY_EXISTS` (Windows); a name that already exists — an orphan left by a crashed same-PID process — is surfaced as an error (`MORI_EEXIST`), never worked around by reusing or mutating the existing region. `prune_shared()` is the mechanism for clearing such orphans; it must run while the PID is free (before reuse), since the colliding process reads its own PID as alive and so cannot reap them itself. The host writes the full region inside `mori_create` before returning, and consumers obtain the name only after that call returns — partial writes are not observable. Consumer mappings are read-only (`PROT_READ` / `FILE_MAP_READ`); mutations trigger COW into private memory. No locking is implemented or required; any future change admitting in-place mutation breaks this model. +mori is **write-once on host, read-many on consumers, COW for any mutation.** Each `share()` allocates a new SHM region with a unique name; existing regions are never mutated. The name embeds the creator PID (`mori__`); the counter starts at a per-process random value (seeded on first use in `mori_shm_name`), so colliding with a dead same-PID incarnation's names additionally requires a ~2^-32 counter overlap; `mori_shm_create` opens with `O_EXCL` (POSIX) / checks `ERROR_ALREADY_EXISTS` (Windows); a name that already exists — an orphan left by a crashed same-PID process — is surfaced as an error (`MORI_EEXIST`), never worked around by reusing or mutating the existing region. `prune_shared()` is the mechanism for clearing such orphans; it must run while the PID is free (before reuse), since the colliding process reads its own PID as alive and so cannot reap them itself. The host writes the full region inside `mori_create` before returning, and consumers obtain the name only after that call returns — partial writes are not observable. Consumer mappings are read-only (`PROT_READ` / `FILE_MAP_READ`); mutations trigger COW into private memory. No locking is implemented or required; any future change admitting in-place mutation breaks this model. Fork safety: `mori_shm` records the creator PID at create time and `mori_host_finalizer` unlinks only when it matches `getpid()` — a fork()ed child's inherited finalizers skip the unlink (the child's `munmap` via `mori_shm_finalizer` is process-local and safe), so `parallel::mclapply`-style forks cannot destroy a parent's live regions; regions orphaned by a dead parent remain reapable by `prune_shared()` as usual. ## share() Dispatch (`altrep.c: mori_create`) @@ -49,7 +50,7 @@ Writing into SHM is count-then-write in two flavours: `mori_serialize_count` / ` ## ALTREP Classes -Registered in `mori_altrep_init`. Numeric types — `mori_real`, `mori_integer`, `mori_logical`, `mori_raw`, `mori_complex` — share the `mori_vec` pattern with a SHM-backed data pointer. `mori_string` uses `mori_str` with lazy per-element access via `Elt` and `Rf_mkCharLenCE`. `mori_list` uses `mori_list_view` with a per-element directory and a lazy `Elt` cache that uses `R_NilValue` as the "uncached" sentinel — a fresh VECSXP is naturally R_NilValue-filled, so cache init is zero work. NIL-valued elements are the sole singleton output of `mori_unwrap_element`'s fallback path, so they aren't cached; re-extraction returns the same `R_NilValue` and identity is preserved for free. +Registered in `mori_altrep_init`. Numeric types — `mori_real`, `mori_integer`, `mori_logical`, `mori_raw`, `mori_complex` — share the `mori_vec` pattern with a SHM-backed data pointer. `mori_string` uses `mori_str` with lazy per-element access via `Elt` and `Rf_mkCharLenCE`; `mori_str` records the packed string area size (`str_bytes`), and each offset-table entry is bounds-checked against it (and the encoding range-checked) at `Elt` time. `mori_list` uses `mori_list_view` with a per-element directory and a lazy `Elt` cache that uses `R_NilValue` as the "uncached" sentinel — a fresh VECSXP is naturally R_NilValue-filled, so cache init is zero work. NIL-valued elements are the sole singleton output of `mori_unwrap_element`'s fallback path, so they aren't cached; re-extraction returns the same `R_NilValue` and identity is preserved for free. ### COW invariant for new ALTREP methods @@ -60,12 +61,14 @@ Any method returning a writable pointer (or mutating) **must check `R_altrep_dat All classes register `Serialized_state` and `Unserialize`. - **Identifier wire form (single canonical shape)**. `mori_format_chain` is the single source of truth used by both `mori_shm_name` (the `.Call`) and the three `Serialized_state` methods. Walks the keeper chain through `mori_owned_tag` hops collecting `view->index` (`>= 0` only) and formats `` (root) or `[i1+1,...]` (sub-object) into a caller-provided stack buffer. `shm->name_len` is cached at create/open time (no per-call `strlen`). 1-based externalisation cues R's `[[i]]` semantics; the parser converts back to 0-based. -- **Unserialize**. `mori_Unserialize` delegates STRSXP × 1 state to `mori_shm_open_and_wrap`; on parse-success it returns the opened ALTREP, on parse-failure it falls through to expanded-state handling (the COW-materialized ALTSTRING case). `mori_shm_open_and_wrap` runs `mori_parse_identifier`: prefix-only routes through magic-dispatch; path form routes through `mori_open_path_c`, which walks each intermediate VECSXP directory via `mori_make_view_extptr` (bare keeper-chain extptr — no ALTLIST wrapper, no attribute restoration, since intermediates are never observed) and calls `mori_unwrap_element` at the leaf. +- **Unserialize**. Two methods: the shared `mori_Unserialize` (vec + list classes) and `mori_string_Unserialize`. For vec/list, a STRSXP state is always an identifier (their materialized fallbacks are atomic vectors / VECSXP, never STRSXP) — it is probed via `mori_shm_open_and_wrap` and a miss errors (corrupt stream); any other state is materialized data returned as-is. For strings both forms are string-like, so the materialized fallback is wrapped in a length-1 VECSXP (`mori_wrap_string_state`): a bare STRSXP state is always an identifier, and the wrapper — whose content could itself parse as an identifier — is unwrapped without probing. `mori_shm_open_and_wrap` runs `mori_parse_identifier`: prefix-only routes through magic-dispatch; path form routes through `mori_open_path_c`, which walks each intermediate VECSXP directory via `mori_make_view_extptr` (bare keeper-chain extptr — no ALTLIST wrapper, no attribute restoration, since intermediates are never observed) and calls `mori_unwrap_element` at the leaf. Fallback to full materialization when: - COW-materialized vectors (data2 is set) — returns the materialized copy. - Nesting depth exceeds `MORI_MAX_PATH` (64) in `mori_format_chain`. +String fallbacks are wrapped in a length-1 VECSXP (`mori_wrap_string_state`) to keep the wire forms disjoint; vec/list fallbacks need no wrapper (their states are never STRSXP). + ### Identifier grammar ``` @@ -81,7 +84,7 @@ int1 ::= [1-9][0-9]* # 1-based, no leading zeros ### Validation contract -`map_shared` and unserialize share `mori_parse_identifier` for shape validation but differ in failure mode: **malformed input → `NULL`** (wrong type/length, `NA`, missing or malformed prefix, malformed bracketed path); **well-formed identifier that fails to map → error** (missing region, bad magic, truncated header, OOB path index, non-VECSXP intermediate). Preserve this split in `mori_shm_open_and_wrap`: collapsing it either way breaks the probe-vs-corruption distinction. +`map_shared` and unserialize share `mori_parse_identifier` for shape validation but differ in failure mode: **malformed input → `NULL`** (wrong type/length, `NA`, missing or malformed prefix, malformed bracketed path); **well-formed identifier that fails to map → error** (missing region, bad magic, truncated header, OOB path index, non-VECSXP intermediate, fields inconsistent with the mapped size). Consumer-side validation is total — corrupt input errors, never reads out of bounds: root headers are checked against the mapped size (`mori_open_vector` / `mori_open_string`), directory entries against their data region (`mori_unwrap_element`: `length × elt_size ≤ data_size − attrs_size`, `attrs_size >= 0`), string tables must fit their region (`mori_make_string`), and each offset-table entry is bounds-checked at `Elt` time. Preserve this split in `mori_shm_open_and_wrap`: collapsing it either way breaks the probe-vs-corruption distinction. ## SHM Region Layouts @@ -134,7 +137,7 @@ Each offset table entry (16 bytes): `str_offset(int64) + str_length(int32) + str SHM lifetime is fully automatic via chained external pointer finalizers. Three interned symbols (C globals in `altrep.c`): - **`mori_shm_tag`** (`Rf_install("mori_shm")`) — tags the SHM mapping extptr (host- and consumer-side). Addr is `mori_shm *`; finalizer: `munmap`. -- **`mori_host_tag`** (`Rf_install("mori_host")`) — tags the host-only unlink extptr created by `mori_make_result`. Addr is `mori_shm *` carrying `name` (POSIX) or `HANDLE` (Windows); finalizer: `shm_unlink` / `CloseHandle`. Sits above the shm terminus in the keeper chain. +- **`mori_host_tag`** (`Rf_install("mori_host")`) — tags the host-only unlink extptr created by `mori_make_result`. Addr is `mori_shm *` carrying `name` (POSIX) or `HANDLE` (Windows); finalizer: `shm_unlink` / `CloseHandle`, skipped when the stored creator `pid` no longer matches `getpid()` (fork guard). Sits above the shm terminus in the keeper chain. - **`mori_owned_tag`** (`Rf_install("mori_owned")`) — tags every malloc-backed extptr in the keeper chain: each mori ALTREP's `data1` extptr (view, vec, or str) **and** the bare keeper-chain extptrs created by `mori_make_view_extptr` for path-walk intermediates (which have no ALTREP wrapper but still hold a `mori_list_view *`). Addr type for ALTREP data1 is recovered from `TYPEOF(x)` at the ALTREP boundary: `VECSXP → mori_list_view *`, `STRSXP → mori_str *`, else `mori_vec *`. Bare keeper-chain hops are always `mori_list_view *` (vec/str only ever appear as leaves). Standalone-vs-element carried in each struct's `index` field (`-1` standalone/root, `>= 0` element/sub-list); redundantly recoverable from `data1.prot`'s tag (`shm_tag → standalone`, `owned_tag → element/sub-list`). Common finalizer: `mori_owned_finalizer` `free`s the addr. `is_shared()` is a pointer comparison against `mori_owned_tag`. `mori_shm_name()` invokes `mori_format_chain` to emit the bare prefix for roots / standalones and the path-bearing form (`[i1,i2,...]`, 1-based) for sub-objects. @@ -149,7 +152,7 @@ SHM lifetime is fully automatic via chained external pointer finalizers. Three i ### src/ -- **mori.h** — types (`mori_shm`, `mori_buf`, `mori_vec` with `index` field, `mori_list_view`), declarations, `MORI_ALIGN64`, `mori_sizeof_elt`, and the identifier grammar constants (`MORI_NAME_MAX`, `MORI_MAX_PATH`, `MORI_IDENTIFIER_MAX`, `MORI_FORMAT_BUFLEN`, `MORI_PREFIX_LITERAL`). +- **mori.h** — types (`mori_shm` with creator `pid` fork guard, `mori_buf`, `mori_vec` with `index` field, `mori_str` with `str_bytes` bound, `mori_list_view`), declarations, `MORI_ALIGN64`, `mori_sizeof_elt`, and the identifier grammar constants (`MORI_NAME_MAX`, `MORI_MAX_PATH`, `MORI_IDENTIFIER_MAX`, `MORI_FORMAT_BUFLEN`, `MORI_PREFIX_LITERAL`). - **shm.c** — platform SHM create/open/close + finalizers for both sides. `mori_shm_reap` enumerates the platform's name source (`#ifdef`-selected) — `/dev/shm` on Linux (entries are region names), the per-user registry dir on macOS (per-process logs; see Platform Notes) — classifies by the PID embedded in the name via `mori_pid_alive` (Linux unlinks each dead-PID region directly; macOS reads each dead-PID log via `mori_reap_log` and unlinks the regions it names), and feeds the shared per-name core `mori_reap_unlink` (shm_unlink + record-on-reclaim). Windows / other POSIX cannot reap → `NULL` / `*n == 0`. `mori_shm_os_unlink` is the single region-unlink seam (host finalizer, create-fail, reap) and is pure `shm_unlink`; the macOS registry is decoupled from it and managed by `mori_log_append` / `mori_log_release` (see Platform Notes). Reap semantics A, both platforms: a dead-PID region is reported removed only when `shm_unlink` actually reclaimed it, so concurrent reaps never double-report. Error path: `mori_err_classify` (static; native errno / GetLastError → portable `MORI_E*` category) and `mori_err_describe` (category summary + platform remediation hint); `mori_shm_create` / `mori_shm_create_heap` return `MORI_OK` or a failure category, classifying the native code **before** any `close` / `unlink` / `CloseHandle` clobbers `errno` / `GetLastError`. - **serialize.c** — `mori_serialize_count`, `mori_serialize_into`, `mori_unserialize_from`. Used for fallback (non-zero-copy) ALTLIST entries where directory `sexptype == 0`. - **altrep.c** — all ALTREP class definitions, `mori_create` dispatcher, recursive writers (`mori_nested_size` / `mori_nested_write`), consumer open+wrap (`mori_shm_open_and_wrap`), path open (`mori_open_path_c`), view constructors (`mori_make_view_extptr` for bare keeper-chain extptrs, `mori_make_list_view` for full ALTLIST + attrs), identity/name (`mori_is_shared` / `mori_shm_name`), identifier formatter / parser (`mori_format_chain` / `mori_parse_identifier`), serialization hooks, `mori_altrep_init`. The create-failure message lives in `mori_shm_create_failed`, which composes the requested size (`mori_format_bytes`) with `mori_err_describe`'s summary + hint into a single `Rf_error`. @@ -168,7 +171,7 @@ testthat edition 3. Entry point `tests/testthat.R`; tests in `tests/testthat/tes ## Platform Notes - **Linux**: SHM in `/dev/shm/` (tmpfs). Accessed via `open()` / `unlink()` directly to avoid the `-lrt` link dependency that `shm_open` requires. `MAP_POPULATE` pre-faults pages. -- **macOS**: Kernel-backed SHM via `shm_open` / `shm_unlink` (in libc, no extra link flags). `MAP_POPULATE` is a no-op (defined to 0). The kernel POSIX-shm namespace has no filesystem footprint and cannot be enumerated (invisible to `ipcs` / `lsof` once the creator dies), so orphan reaping relies on a per-user registry under `TMPDIR` (resolved via `$TMPDIR`, then `confstr(_CS_DARWIN_USER_TEMP_DIR)`, then `/tmp`) that `mori_shm_reap` scans in place of `/dev/shm`. The registry must live in an enumerable namespace — putting it in SHM would face the same non-enumerability wall — so it is filesystem-backed, but to keep the per-`share()` cost off the critical path it is **one append-only log per process** (`/mori_`) rather than a file per region: `mori_log_append` (called in `mori_shm_create` before the region is observable) opens the log lazily on first share and appends one line per region name — a single `write()` (~1 µs) vs creating a file (~40 µs on APFS, where neither path resolution nor a cached dir fd helps because the cost is inode allocation + journaling). Single-writer per process ⇒ no locking; the only reader is a reaper of a *dead* PID, which by definition has no concurrent writer. `mori_log_release` (host finalizer / create-fail) decrements a process-global live-region count and, at zero, closes+unlinks the log and `rmdir`s the now-empty dir — so the dir tracks the live-region set as the per-region markers did, but driven by finalization. The count is incremented unconditionally so it stays balanced when logging itself fails (which forfeits only reapability, never the region). Fork-safe: `mori_log_fork_guard` reopens the log when `getpid()` changes (the inherited fd is the parent's — closed, never unlinked). Crash semantics match the regions': a `write()` is visible cross-process via the page cache without `fsync` and survives the writer's death, lost only on reboot. Path resolution (`mori_log_dir`) is pure — builds/caches the path but never creates the dir (`mori_log_append` retries through `mkdir` on `ENOENT` and is the sole creator), so a reap on a process with no log opens nothing and leaves nothing behind. All log ops best-effort. +- **macOS**: Kernel-backed SHM via `shm_open` / `shm_unlink` (in libc, no extra link flags). `MAP_POPULATE` is a no-op (defined to 0). The kernel POSIX-shm namespace has no filesystem footprint and cannot be enumerated (invisible to `ipcs` / `lsof` once the creator dies), so orphan reaping relies on a per-user registry under `TMPDIR` (resolved via `$TMPDIR`, then `confstr(_CS_DARWIN_USER_TEMP_DIR)`, then `/tmp`) that `mori_shm_reap` scans in place of `/dev/shm`. The registry must live in an enumerable namespace — putting it in SHM would face the same non-enumerability wall — so it is filesystem-backed, but to keep the per-`share()` cost off the critical path it is **one append-only log per process** (`/mori_`) rather than a file per region: `mori_log_append` (called in `mori_shm_create` before the region is observable) opens the log lazily on first share and appends one line per region name — a single `write()` (~1 µs) vs creating a file (~40 µs on APFS, where neither path resolution nor a cached dir fd helps because the cost is inode allocation + journaling). Single-writer per process ⇒ no locking; the only reader is a reaper of a *dead* PID, which by definition has no concurrent writer. `mori_log_release` (host finalizer / create-fail) decrements a process-global live-region count and, at zero, closes+unlinks the log and `rmdir`s the now-empty dir — so the dir tracks the live-region set as the per-region markers did, but driven by finalization. The count is incremented unconditionally so it stays balanced when logging itself fails (which forfeits only reapability, never the region). Fork-safe: `mori_log_fork_guard` reopens the log when `getpid()` changes (the inherited fd is the parent's — closed, never unlinked). The region unlink in `mori_host_finalizer` is fork-guarded the same way (creator PID stored in `mori_shm`; see Concurrency Model). Crash semantics match the regions': a `write()` is visible cross-process via the page cache without `fsync` and survives the writer's death, lost only on reboot. Path resolution (`mori_log_dir`) is pure — builds/caches the path but never creates the dir (`mori_log_append` retries through `mkdir` on `ENOENT` and is the sole creator), so a reap on a process with no log opens nothing and leaves nothing behind. All log ops best-effort. - **Windows**: Page-file-backed via `CreateFileMappingA` / `MapViewOfFile`. `kernel32` is always available. Host must keep the mapping handle alive until consumers have opened it (the GC-chained host extptr handles this). ## Package Conventions diff --git a/NEWS.md b/NEWS.md index eb548d5..0d8b6ab 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,9 @@ # mori (development version) +* Fixed type confusion on unserialize when a shared string's content resembled a shared memory identifier (e.g. a stored `shared_name()` value). +* Fixed a forked child process (e.g. `parallel::mclapply`) unlinking the parent's live region when garbage-collecting an inherited shared object. +* Corrupted regions now raise a clean R error on `map_shared()` or access instead of reading out of bounds or crashing R. + # mori 0.2.2 * Region name counters now start at a per-process random value, so a process reusing a crashed process's PID no longer collides with its orphaned regions (#50). diff --git a/src/altrep.c b/src/altrep.c index ad6fa90..d3c0d58 100644 --- a/src/altrep.c +++ b/src/altrep.c @@ -360,6 +360,7 @@ typedef struct { const unsigned char *table; const unsigned char *data; R_xlen_t length; + int64_t str_bytes; /* size of the packed string area; bounds each entry */ int32_t index; /* -1 = standalone, >= 0 = element of ALTLIST */ } mori_str; @@ -370,6 +371,12 @@ static inline SEXP mori_string_elt_shm(mori_str *s, R_xlen_t i) { if (e.str_length < 0) return NA_STRING; + /* Bounds-check the entry against the packed string area before reading */ + if (e.str_offset < 0 || + e.str_offset > s->str_bytes - (int64_t) e.str_length || + e.str_encoding < CE_NATIVE || e.str_encoding > CE_BYTES) + Rf_error("mori: invalid string data"); + return Rf_mkCharLenCE((const char *) (s->data + e.str_offset), e.str_length, (cetype_t) e.str_encoding); } @@ -424,18 +431,30 @@ static SEXP mori_string_Duplicate(SEXP x, Rboolean deep) { return result; } -/* region_base: points to the offset table. +/* region_base: points to the offset table. data_size: bytes available for + the table, alignment padding, and packed strings (the caller excludes any + trailing attributes) — the table must fit within it, and the remainder is + recorded as str_bytes to bound each offset-table entry at Elt time. keeper: SEXP kept alive via the extptr's protected slot (parent SHM). */ static SEXP mori_make_string(const unsigned char *region_base, - R_xlen_t n, SEXP keeper) { + R_xlen_t n, int64_t data_size, SEXP keeper) { + + if (n < 0 || data_size < 0 || + n > data_size / (R_xlen_t) sizeof(mori_str_entry)) + Rf_error("mori: invalid string data"); + + size_t table_size = sizeof(mori_str_entry) * (size_t) n; + size_t aligned = MORI_ALIGN64(table_size); + if (aligned > (size_t) data_size) + Rf_error("mori: invalid string data"); mori_str *s = malloc(sizeof(mori_str)); if (s == NULL) Rf_error("mori: allocation failure"); - size_t table_size = sizeof(mori_str_entry) * (size_t) n; s->table = region_base; - s->data = region_base + MORI_ALIGN64(table_size); + s->data = region_base + aligned; s->length = n; + s->str_bytes = data_size - (int64_t) aligned; s->index = -1; SEXP ptr = PROTECT(R_MakeExternalPtr(s, mori_owned_tag, keeper)); @@ -465,7 +484,7 @@ static SEXP mori_unwrap_element(unsigned char *base, int64_t region_size, if (mori_oob(data_offset, data_size, region_size)) Rf_error("mori: invalid element data"); - if (attrs_size > 0 && attrs_size > data_size) + if (attrs_size < 0 || attrs_size > data_size) Rf_error("mori: invalid element data"); SEXP result; @@ -476,10 +495,17 @@ static SEXP mori_unwrap_element(unsigned char *base, int64_t region_size, )); } else if (sexptype == STRSXP) { result = PROTECT(mori_make_string( - base + data_offset, (R_xlen_t) length, keeper + base + data_offset, (R_xlen_t) length, data_size - attrs_size, keeper )); ((mori_str *) R_ExternalPtrAddr(R_altrep_data1(result)))->index = index; } else if (sexptype != 0) { + /* The claimed element data must fit within the directory entry's data + region (minus trailing attributes) */ + size_t elt_size = mori_sizeof_elt(sexptype); + if (elt_size != 0 && + (length < 0 || + length > (data_size - attrs_size) / (int64_t) elt_size)) + Rf_error("mori: invalid element data"); result = PROTECT(mori_make_vector( base + data_offset, (R_xlen_t) length, sexptype, keeper )); @@ -1037,6 +1063,16 @@ static SEXP mori_open_vector(SEXP shm_ptr) { memcpy(&length, base + 8, 8); memcpy(&attrs_size, base + 16, 8); + /* Validate the header against the mapped size before any data access: + the data block and trailing attributes must fit within the region. + Short-circuit order keeps the length * elt_size product overflow-free. */ + int64_t region_size = (int64_t) shm->size; + size_t elt_size = mori_sizeof_elt(sexptype); + if (region_size < 64 || length < 0 || attrs_size < 0 || + (elt_size != 0 && length > (region_size - 64) / (int64_t) elt_size) || + attrs_size > region_size - 64 - length * (int64_t) elt_size) + Rf_error("mori: invalid or corrupted shared memory region"); + SEXP result = PROTECT(mori_make_vector( base + 64, (R_xlen_t) length, sexptype, shm_ptr )); @@ -1060,8 +1096,16 @@ static SEXP mori_open_string(SEXP shm_ptr) { memcpy(&n, base + 8, 8); memcpy(&str_data_size, base + 16, 8); + /* Validate the header against the mapped size before any data access: + the string data and trailing attributes must fit within the region. */ + int64_t region_size = (int64_t) shm->size; + if (region_size < 24 || n < 0 || str_data_size < 0 || attrs_size < 0 || + str_data_size > region_size - 24 || + attrs_size > region_size - 24 - str_data_size) + Rf_error("mori: invalid or corrupted shared memory region"); + SEXP result = PROTECT(mori_make_string( - base + 24, (R_xlen_t) n, shm_ptr + base + 24, (R_xlen_t) n, str_data_size, shm_ptr )); if (attrs_size > 0) @@ -1186,7 +1230,9 @@ SEXP mori_prune(void) { mori_shm_name (the .Call): bare prefix for root standalones, prefix + bracketed 1-based path for sub-objects. mori_format_chain is the single source of truth shared with mori_shm_name. On overflow / malformed - chain, fall back to materialization. */ + chain, fall back to materialization. The string class wraps materialized + states in a length-1 VECSXP (see mori_wrap_string_state) so that a bare + STRSXP state is always an identifier. */ static SEXP mori_vec_Serialized_state(SEXP x) { SEXP data2 = R_altrep_data2(x); @@ -1209,9 +1255,21 @@ static SEXP mori_vec_Serialized_state(SEXP x) { return mat; } +/* Wrap a materialized string vector in a length-1 VECSXP so the wire form + is unambiguous: a bare STRSXP state is always an SHM identifier, and the + materialized data — whose content could itself look like an identifier — + is never mistaken for one. The fallback paths are cold (COW or nesting + beyond MORI_MAX_PATH), so the wrapper costs nothing on the hot path. */ +static SEXP mori_wrap_string_state(SEXP x) { + SEXP state = PROTECT(Rf_allocVector(VECSXP, 1)); + SET_VECTOR_ELT(state, 0, x); + UNPROTECT(1); + return state; +} + static SEXP mori_string_Serialized_state(SEXP x) { SEXP data2 = R_altrep_data2(x); - if (data2 != R_NilValue) return data2; /* COW-materialized copy */ + if (data2 != R_NilValue) return mori_wrap_string_state(data2); SEXP data1 = R_altrep_data1(x); mori_str *s = (mori_str *) R_ExternalPtrAddr(data1); @@ -1226,8 +1284,9 @@ static SEXP mori_string_Serialized_state(SEXP x) { SEXP mat = PROTECT(Rf_allocVector(STRSXP, n)); for (R_xlen_t i = 0; i < n; i++) SET_STRING_ELT(mat, i, mori_string_elt_shm(s, i)); + SEXP state = mori_wrap_string_state(mat); UNPROTECT(1); - return mat; + return state; } static SEXP mori_list_Serialized_state(SEXP x) { @@ -1319,28 +1378,39 @@ static SEXP mori_open_path_c(const char *name, return result; } +/* Vec and list classes: a STRSXP state is always an SHM identifier (their + materialized fallbacks are atomic vectors or VECSXP, never STRSXP), so a + probe miss means a corrupt stream. A well-formed identifier whose region + is gone errors inside mori_shm_open_and_wrap (corruption, not user + data). Any other state is materialized data, returned as-is (R restores + ALTREP attributes separately). The string class has its own method + below, since both of its wire forms are string-like. */ static SEXP mori_Unserialize(SEXP class_info, SEXP state) { (void) class_info; - /* STRSXP state has two legitimate sources: an SHM identifier (root or - path-bearing) emitted by mori_*_Serialized_state, or a COW-materialized - ALTSTRING handed back as itself. mori_shm_open_and_wrap distinguishes - them by parse: parse success + region opens → opened ALTREP returned; - parse success + region missing → Rf_error propagates (correct: a - well-formed identifier whose region is gone is corruption, not user - data); parse failure → NULL, fall through to expanded-state handling - for the materialized-ALTSTRING case. The fallthrough is only for the - parse-failure branch — do not widen it to swallow errors from the - parse-success-but-missing-region branch. */ - if (TYPEOF(state) == STRSXP && XLENGTH(state) == 1 && - STRING_ELT(state, 0) != NA_STRING) { + if (TYPEOF(state) == STRSXP) { SEXP opened = mori_shm_open_and_wrap(state); if (opened != R_NilValue) return opened; + Rf_error("mori: invalid serialized state for a shared object"); } - /* Expanded state: materialized data → return as-is - (R restores ALTREP attributes separately) */ return state; } +/* String class: the identifier form is a bare STRSXP; the materialized + fallback is wrapped in a length-1 VECSXP (see mori_wrap_string_state). + The forms are disjoint by construction, so anything else is a corrupt + stream. */ +static SEXP mori_string_Unserialize(SEXP class_info, SEXP state) { + (void) class_info; + if (TYPEOF(state) == VECSXP && XLENGTH(state) == 1 && + TYPEOF(VECTOR_ELT(state, 0)) == STRSXP) + return VECTOR_ELT(state, 0); + if (TYPEOF(state) == STRSXP) { + SEXP opened = mori_shm_open_and_wrap(state); + if (opened != R_NilValue) return opened; + } + Rf_error("mori: invalid serialized state for a shared string vector"); +} + // ALTREP class registration --------------------------------------------------- typedef R_altrep_class_t (*mori_make_class_fn)(const char *, const char *, @@ -1400,5 +1470,5 @@ void mori_altrep_init(DllInfo *dll) { R_set_altstring_Elt_method(mori_string_class, mori_string_Elt); R_set_altrep_Serialized_state_method(mori_string_class, mori_string_Serialized_state); - R_set_altrep_Unserialize_method(mori_string_class, mori_Unserialize); + R_set_altrep_Unserialize_method(mori_string_class, mori_string_Unserialize); } diff --git a/src/mori.h b/src/mori.h index 9971ad8..6bfd978 100644 --- a/src/mori.h +++ b/src/mori.h @@ -10,7 +10,7 @@ // Identifier grammar constants ------------------------------------------------ -#define MORI_NAME_MAX 30 /* size of mori_shm.name; fits Windows worst case (Local\\mori_<8hex>_<8hex> = 28) + NUL with 1 byte slack, and lands name_len in mori_shm's natural tail padding (sizeof = 48 POSIX / 56 Windows) */ +#define MORI_NAME_MAX 30 /* size of mori_shm.name; fits Windows worst case (Local\\mori_<8hex>_<8hex> = 28) + NUL with 1 byte slack */ #define MORI_MAX_PATH 64 /* max indices in a path */ #define MORI_IDENTIFIER_MAX 1024 /* parser input length cap */ #define MORI_FORMAT_BUFLEN 1024 /* formatter stack buffer */ @@ -28,6 +28,7 @@ typedef struct mori_shm_s { size_t size; char name[MORI_NAME_MAX]; uint8_t name_len; /* strlen(name); fits since MORI_NAME_MAX < 256 */ + unsigned int pid; /* creator PID: fork guard for the host finalizer (read on POSIX only; Windows has no fork) */ #ifdef _WIN32 void *handle; #endif diff --git a/src/shm.c b/src/shm.c index f8701fc..c5822aa 100644 --- a/src/shm.c +++ b/src/shm.c @@ -38,19 +38,19 @@ static int mori_err_classify(long code) { } } -static size_t mori_shm_name(char *name, size_t size) { +static size_t mori_shm_name(char *name, size_t size, unsigned int pid) { static unsigned int counter; static int seeded; if (!seeded) { LARGE_INTEGER t; QueryPerformanceCounter(&t); counter = mori_counter_seed((uint64_t) t.QuadPart ^ - ((uint64_t) GetCurrentProcessId() << 40) ^ + ((uint64_t) pid << 40) ^ (uint64_t) (uintptr_t) &counter); seeded = 1; } int n = snprintf(name, size, MORI_PREFIX_LITERAL "%lx_%x", - (unsigned long) GetCurrentProcessId(), counter++); + (unsigned long) pid, counter++); return (n > 0 && (size_t) n < size) ? (size_t) n : 0; } @@ -59,11 +59,12 @@ int mori_shm_create(mori_shm *shm, size_t size) { shm->addr = NULL; shm->size = 0; shm->handle = NULL; + shm->pid = (unsigned int) GetCurrentProcessId(); DWORD hi = (DWORD) ((uint64_t) size >> 32); DWORD lo = (DWORD) (size & 0xFFFFFFFF); - shm->name_len = (uint8_t) mori_shm_name(shm->name, sizeof(shm->name)); + shm->name_len = (uint8_t) mori_shm_name(shm->name, sizeof(shm->name), shm->pid); HANDLE h = CreateFileMappingA( INVALID_HANDLE_VALUE, NULL, PAGE_READWRITE, hi, lo, shm->name ); @@ -96,6 +97,7 @@ int mori_shm_open(mori_shm *shm, const char *name) { memcpy(shm->name, name, nl); shm->name[nl] = '\0'; shm->name_len = (uint8_t) nl; + shm->pid = 0; /* consumer: never the creator */ HANDLE h = OpenFileMappingA(FILE_MAP_READ, FALSE, name); if (h == NULL) return -1; @@ -432,7 +434,7 @@ char **mori_shm_reap(int *n) { #endif /* __linux__ || __APPLE__ */ -static size_t mori_shm_name(char *name, size_t size) { +static size_t mori_shm_name(char *name, size_t size, unsigned int pid) { static unsigned int counter; static int seeded; if (!seeded) { @@ -440,12 +442,11 @@ static size_t mori_shm_name(char *name, size_t size) { clock_gettime(CLOCK_MONOTONIC, &ts); counter = mori_counter_seed(((uint64_t) ts.tv_sec << 32) ^ (uint64_t) ts.tv_nsec ^ - ((uint64_t) getpid() << 40) ^ + ((uint64_t) pid << 40) ^ (uint64_t) (uintptr_t) &counter); seeded = 1; } - int n = snprintf(name, size, MORI_PREFIX_LITERAL "%x_%x", - (unsigned) getpid(), counter++); + int n = snprintf(name, size, MORI_PREFIX_LITERAL "%x_%x", pid, counter++); return (n > 0 && (size_t) n < size) ? (size_t) n : 0; } @@ -470,7 +471,8 @@ int mori_shm_create(mori_shm *shm, size_t size) { shm->addr = NULL; shm->size = 0; - shm->name_len = (uint8_t) mori_shm_name(shm->name, sizeof(shm->name)); + shm->pid = (unsigned int) getpid(); + shm->name_len = (uint8_t) mori_shm_name(shm->name, sizeof(shm->name), shm->pid); int fd = mori_shm_os_open(shm->name, O_CREAT | O_EXCL | O_RDWR, 0600); if (fd < 0) return errno == EEXIST ? MORI_EEXIST : mori_err_classify(errno); @@ -520,6 +522,7 @@ int mori_shm_open(mori_shm *shm, const char *name) { memcpy(shm->name, name, nl); shm->name[nl] = '\0'; shm->name_len = (uint8_t) nl; + shm->pid = 0; /* consumer: never the creator */ int fd = mori_shm_os_open(name, O_RDONLY, 0); if (fd < 0) return -1; @@ -643,7 +646,11 @@ void mori_host_finalizer(SEXP ptr) { #ifdef _WIN32 if (shm->handle != NULL) CloseHandle(shm->handle); #else - if (shm->name[0] != '\0') mori_shm_os_unlink(shm->name); + /* Unlink only in the creating process: a fork()ed child inherits this + finalizer for the parent's regions and must not destroy their names + (its own munmap via mori_shm_finalizer is process-local and safe). */ + if (shm->name[0] != '\0' && shm->pid == (unsigned int) getpid()) + mori_shm_os_unlink(shm->name); #ifdef __APPLE__ mori_log_release(); /* balance the create-time append */ #endif diff --git a/tests/testthat/test-corruption.R b/tests/testthat/test-corruption.R index 5b6a89f..db10f18 100644 --- a/tests/testthat/test-corruption.R +++ b/tests/testthat/test-corruption.R @@ -17,15 +17,40 @@ i64 <- function(x) { out } -MORL <- strtoi("4d4f524c", 16L) # "MORL" magic, little-endian +MORL <- strtoi("4d4f524c", 16L) # "MORL" magic, little-endian +MORH <- strtoi("4d4f5248", 16L) # "MORH" magic, little-endian +MORS <- strtoi("4d4f5253", 16L) # "MORS" magic, little-endian REALSXP <- 14L +STRSXP <- 16L VECSXP <- 19L # MORL header (24 bytes) + 32-byte directory entries (see SHM Region Layouts). -morl_header <- function(n, attrs_offset = 0, attrs_size = 0) +morl_header <- function(n, attrs_offset = 0, attrs_size = 0) { c(i32(MORL), i32(n), i64(attrs_offset), i64(attrs_size)) -morl_entry <- function(data_offset, data_size, sexptype, attrs_size = 0, length = 0) - c(i64(data_offset), i64(data_size), i32(sexptype), i32(attrs_size), i64(length)) +} +morl_entry <- function( + data_offset, + data_size, + sexptype, + attrs_size = 0, + length = 0 +) { + c( + i64(data_offset), + i64(data_size), + i32(sexptype), + i32(attrs_size), + i64(length) + ) +} + +# MORH header (64 bytes) and MORS header (24 bytes). +morh_header <- function(sexptype, length, attrs_size = 0) { + c(i32(MORH), i32(sexptype), i64(length), i64(attrs_size), raw(40L)) +} +mors_header <- function(n, str_data_size, attrs_size = 0) { + c(i32(MORS), i32(attrs_size), i64(n), i64(str_data_size)) +} # Write `bytes` to a fresh /dev/shm region with a well-formed mori name, arrange # for its removal when the calling test exits, and return the identifier. The @@ -45,55 +70,71 @@ write_corrupt <- function(bytes, envir = parent.frame()) { } test_that("map_shared() errors on an unknown magic number", { - if (Sys.info()[["sysname"]] != "Linux") + if (Sys.info()[["sysname"]] != "Linux") { skip("requires file-backed /dev/shm (Linux only)") + } - name <- write_corrupt(c(i32(0L), raw(60L))) # 64-byte region, magic = 0 + name <- write_corrupt(c(i32(0L), raw(60L))) # 64-byte region, magic = 0 expect_error(map_shared(name), "invalid or corrupted") }) test_that("map_shared() errors on a truncated MORL header", { - if (Sys.info()[["sysname"]] != "Linux") + if (Sys.info()[["sysname"]] != "Linux") { skip("requires file-backed /dev/shm (Linux only)") + } - name <- write_corrupt(c(i32(MORL), raw(4L))) # magic present, header < 24 bytes + name <- write_corrupt(c(i32(MORL), raw(4L))) # magic present, header < 24 bytes expect_error(map_shared(name), "invalid nested list region") }) test_that("map_shared() errors on an out-of-range element count", { - if (Sys.info()[["sysname"]] != "Linux") + if (Sys.info()[["sysname"]] != "Linux") { skip("requires file-backed /dev/shm (Linux only)") + } - name <- write_corrupt(morl_header(n = 1000L)) # 24-byte region claims 1000 elements + name <- write_corrupt(morl_header(n = 1000L)) # 24-byte region claims 1000 elements expect_error(map_shared(name), "invalid nested list region") }) test_that("element access errors when a directory offset is out of bounds", { - if (Sys.info()[["sysname"]] != "Linux") + if (Sys.info()[["sysname"]] != "Linux") { skip("requires file-backed /dev/shm (Linux only)") + } name <- write_corrupt(c( morl_header(n = 1L), - morl_entry(data_offset = 2^40, data_size = 8, sexptype = REALSXP, length = 1) + morl_entry( + data_offset = 2^40, + data_size = 8, + sexptype = REALSXP, + length = 1 + ) )) expect_error(map_shared(name)[[1]], "invalid element data") }) test_that("element access errors when attrs exceed the element data", { - if (Sys.info()[["sysname"]] != "Linux") + if (Sys.info()[["sysname"]] != "Linux") { skip("requires file-backed /dev/shm (Linux only)") + } name <- write_corrupt(c( morl_header(n = 1L), - morl_entry(data_offset = 24, data_size = 8, sexptype = REALSXP, - attrs_size = 16, length = 1) + morl_entry( + data_offset = 24, + data_size = 8, + sexptype = REALSXP, + attrs_size = 16, + length = 1 + ) )) expect_error(map_shared(name)[[1]], "invalid element data") }) test_that("nested element access errors on a corrupt child region", { - if (Sys.info()[["sysname"]] != "Linux") + if (Sys.info()[["sysname"]] != "Linux") { skip("requires file-backed /dev/shm (Linux only)") + } # parent (n = 1) points element 0 at a 24-byte VECSXP child with bad magic name <- write_corrupt(c( @@ -105,8 +146,9 @@ test_that("nested element access errors on a corrupt child region", { }) test_that("path-form map_shared() errors on an out-of-bounds intermediate", { - if (Sys.info()[["sysname"]] != "Linux") + if (Sys.info()[["sysname"]] != "Linux") { skip("requires file-backed /dev/shm (Linux only)") + } name <- write_corrupt(c( morl_header(n = 1L), @@ -114,3 +156,156 @@ test_that("path-form map_shared() errors on an out-of-bounds intermediate", { )) expect_error(map_shared(paste0(name, "[1,1]")), "invalid nested region") }) + +test_that("map_shared() errors on an out-of-range vector length", { + if (Sys.info()[["sysname"]] != "Linux") { + skip("requires file-backed /dev/shm (Linux only)") + } + + # 64-byte MORH region claims 2^40 REALSXP elements + name <- write_corrupt(morh_header(REALSXP, length = 2^40)) + expect_error(map_shared(name), "invalid or corrupted") +}) + +test_that("map_shared() errors on a negative vector length", { + if (Sys.info()[["sysname"]] != "Linux") { + skip("requires file-backed /dev/shm (Linux only)") + } + + name <- write_corrupt(morh_header(REALSXP, length = -1)) + expect_error(map_shared(name), "invalid or corrupted") +}) + +test_that("map_shared() errors when vector attributes exceed the region", { + if (Sys.info()[["sysname"]] != "Linux") { + skip("requires file-backed /dev/shm (Linux only)") + } + + # length = 0 so the attrs_size check (not the length check) fires + name <- write_corrupt(morh_header(REALSXP, length = 0, attrs_size = 2^40)) + expect_error(map_shared(name), "invalid or corrupted") +}) + +test_that("map_shared() errors when a string count exceeds its region", { + if (Sys.info()[["sysname"]] != "Linux") { + skip("requires file-backed /dev/shm (Linux only)") + } + + # 24-byte MORS region claims 1000 strings (table alone needs 16000 bytes) + name <- write_corrupt(mors_header(n = 1000, str_data_size = 0)) + expect_error(map_shared(name), "invalid string data") +}) + +test_that("map_shared() errors on a negative string count", { + if (Sys.info()[["sysname"]] != "Linux") { + skip("requires file-backed /dev/shm (Linux only)") + } + + name <- write_corrupt(mors_header(n = -1, str_data_size = 0)) + expect_error(map_shared(name), "invalid or corrupted") +}) + +test_that("map_shared() errors when string data exceeds the region", { + if (Sys.info()[["sysname"]] != "Linux") { + skip("requires file-backed /dev/shm (Linux only)") + } + + name <- write_corrupt(mors_header(n = 0, str_data_size = 2^40)) + expect_error(map_shared(name), "invalid or corrupted") +}) + +test_that("element access errors when a vector length exceeds its data", { + if (Sys.info()[["sysname"]] != "Linux") { + skip("requires file-backed /dev/shm (Linux only)") + } + + # raw(8L) pads the region so the entry's [56, 64) data claim is in bounds + # and the length check (not the offset check) fires + name <- write_corrupt(c( + morl_header(n = 1L), + morl_entry( + data_offset = 56, + data_size = 8, + sexptype = REALSXP, + length = 2^40 + ), + raw(8L) + )) + expect_error(map_shared(name)[[1]], "invalid element data") +}) + +test_that("element access errors on a negative element attrs size", { + if (Sys.info()[["sysname"]] != "Linux") { + skip("requires file-backed /dev/shm (Linux only)") + } + + # raw(8L) pads the region so the entry's [56, 64) data claim is in bounds + # and the attrs_size check (not the offset check) fires + name <- write_corrupt(c( + morl_header(n = 1L), + morl_entry( + data_offset = 56, + data_size = 8, + sexptype = REALSXP, + attrs_size = -1, + length = 1 + ), + raw(8L) + )) + expect_error(map_shared(name)[[1]], "invalid element data") +}) + +test_that("element access errors when a string table exceeds its data", { + if (Sys.info()[["sysname"]] != "Linux") { + skip("requires file-backed /dev/shm (Linux only)") + } + + # raw(8L) pads the region so the entry's [56, 64) data claim is in bounds + # and the string table check (not the offset check) fires + name <- write_corrupt(c( + morl_header(n = 1L), + morl_entry( + data_offset = 56, + data_size = 8, + sexptype = STRSXP, + length = 100 + ), + raw(8L) + )) + expect_error(map_shared(name)[[1]], "invalid string data") +}) + +test_that("string access errors on an out-of-bounds offset table entry", { + if (Sys.info()[["sysname"]] != "Linux") { + skip("requires file-backed /dev/shm (Linux only)") + } + + # Table fits (str_bytes = 0), but the sole entry claims 10 bytes at + # offset 1000 — caught at element access, not at open. + name <- write_corrupt(c( + morl_header(n = 1L), + morl_entry(data_offset = 56, data_size = 64, sexptype = STRSXP, length = 1), + i64(1000), + i32(10), + i32(0), + raw(48L) + )) + s <- map_shared(name)[[1]] + expect_error(s[1], "invalid string data") +}) + +test_that("root string access errors on an out-of-bounds offset table entry", { + if (Sys.info()[["sysname"]] != "Linux") { + skip("requires file-backed /dev/shm (Linux only)") + } + + name <- write_corrupt(c( + mors_header(n = 1, str_data_size = 64), + i64(1000), + i32(10), + i32(0), + raw(48L) + )) + s <- map_shared(name) + expect_error(s[1], "invalid string data") +}) diff --git a/tests/testthat/test-gc.R b/tests/testthat/test-gc.R index 6564118..5f6bc07 100644 --- a/tests/testthat/test-gc.R +++ b/tests/testthat/test-gc.R @@ -36,3 +36,30 @@ test_that("element reference keeps parent SHM alive through GC", { gc() expect_error(map_shared(nm), "not found") }) + +test_that("GC in a forked child does not unlink the parent's region", { + skip_on_os("windows") + # Fork is unavailable in some front-ends (e.g. a Positron session) + fork_ok <- tryCatch( + { + parallel::mccollect(parallel::mcparallel(TRUE)) + TRUE + }, + error = function(e) FALSE + ) + skip_if_not(fork_ok, "fork not available in this session") + + x <- share(1:10) + nm <- shared_name(x) + # The child drops its inherited reference and GCs, running the inherited + # host finalizer; the parent's region name must survive. + job <- parallel::mcparallel({ + rm(x) + gc() + TRUE + }) + expect_identical(parallel::mccollect(job)[[1]], TRUE) + + expect_silent(y <- map_shared(nm)) + expect_identical(y[], 1:10) +}) diff --git a/tests/testthat/test-serialization.R b/tests/testthat/test-serialization.R index 5085121..ae7f92c 100644 --- a/tests/testthat/test-serialization.R +++ b/tests/testthat/test-serialization.R @@ -147,10 +147,14 @@ test_that("factor column extracted from list serializes compactly with attrs", { test_that("vec leaf beyond MORI_MAX_PATH falls back to materialization", { x <- 1:5 - for (i in seq_len(70)) x <- list(x) + for (i in seq_len(70)) { + x <- list(x) + } sx <- share(x) deep <- sx - for (i in seq_len(70)) deep <- deep[[1]] + for (i in seq_len(70)) { + deep <- deep[[1]] + } expect_true(is_shared(deep)) y <- unserialize(serialize(deep, NULL)) expect_identical(y[], 1:5) @@ -158,40 +162,87 @@ test_that("vec leaf beyond MORI_MAX_PATH falls back to materialization", { test_that("string leaf beyond MORI_MAX_PATH falls back to materialization", { x <- c("alpha", "beta", "gamma") - for (i in seq_len(70)) x <- list(x) + for (i in seq_len(70)) { + x <- list(x) + } sx <- share(x) deep <- sx - for (i in seq_len(70)) deep <- deep[[1]] + for (i in seq_len(70)) { + deep <- deep[[1]] + } expect_true(is_shared(deep)) y <- unserialize(serialize(deep, NULL)) expect_identical(y[], c("alpha", "beta", "gamma")) }) test_that("length-1 string leaf beyond MORI_MAX_PATH unserializes as-is", { - # Deep nesting forces Serialized_state to materialize; the resulting - # STRSXP-of-1 enters Unserialize's parse branch, parse fails (the literal - # "alpha" is not a mori identifier), and the fallthrough at the end of - # Unserialize returns it unchanged. + # Deep nesting forces Serialized_state to materialize; the materialized + # STRSXP is wrapped in a length-1 VECSXP so it is never probed as an SHM + # identifier, and mori_string_Unserialize unwraps it. x <- "alpha" - for (i in seq_len(70)) x <- list(x) + for (i in seq_len(70)) { + x <- list(x) + } sx <- share(x) deep <- sx - for (i in seq_len(70)) deep <- deep[[1]] + for (i in seq_len(70)) { + deep <- deep[[1]] + } expect_true(is_shared(deep)) y <- unserialize(serialize(deep, NULL)) expect_identical(y[], "alpha") }) +test_that("deep string leaf whose content is a live identifier round-trips as data", { + # Regression: a materialized length-1 STRSXP whose content parses as a + # mori identifier must not be probed — it previously unserialized to the + # named region's contents (wrong object, wrong type). + holder <- share(1:10) + nm <- shared_name(holder) + x <- nm + for (i in seq_len(70)) { + x <- list(x) + } + sx <- share(x) + deep <- sx + for (i in seq_len(70)) { + deep <- deep[[1]] + } + expect_true(is_shared(deep)) + y <- unserialize(serialize(deep, NULL)) + expect_identical(y, nm) +}) + +test_that("deep string leaf naming an absent region round-trips as data", { + # Regression: this previously errored with "region not found". + x <- "/mori_1_1" + for (i in seq_len(70)) { + x <- list(x) + } + sx <- share(x) + deep <- sx + for (i in seq_len(70)) { + deep <- deep[[1]] + } + expect_identical(unserialize(serialize(deep, NULL)), "/mori_1_1") +}) + test_that("sub-list beyond MORI_MAX_PATH falls back to materialization", { x <- list(leaf = 1:3) - for (i in seq_len(70)) x <- list(x) + for (i in seq_len(70)) { + x <- list(x) + } sx <- share(x) deep <- sx - for (i in seq_len(68)) deep <- deep[[1]] + for (i in seq_len(68)) { + deep <- deep[[1]] + } expect_true(is_shared(deep)) expect_true(is.list(deep)) y <- unserialize(serialize(deep, NULL)) final <- y - while (is.list(final) && is.null(names(final))) final <- final[[1]] + while (is.list(final) && is.null(names(final))) { + final <- final[[1]] + } expect_identical(final$leaf[], 1:3) })