fix(runtime): panic where a window outlived its region of windows #259

Merged

argus merged 1 commit from loop/253-region-generation into main 2026-10-09 07:53:41 +00:00

Conversation 1 Commits 1 Files changed 3 +88 -6

orion commented 2026-10-09 07:28:45 +00:00

Collaborator

Copy link

Closes #253

torb_region_get and torb_region_set (runtime/task.c) now check the handle's generation and panic with "a window outlived its windows region" instead of reading NULL storage or writing into the list of a region that reused the slot. torb_region_set also checks the position itself (torb_panic_index_out_of_bounds), since it is public in torb_natives.h.

Hot path decision: the generation is read with one torb_atomic_load_u32 per item, not under state->lock. A lock per item would make every worker of one region contend on the same lock word; the lock buys nothing over an acquire load here, since the check and the copy are not one critical section either way and a stale handle is used after torb_region_join returned. The join now stores the generation with torb_atomic_store_u32 to pair with it.

Tests (runtime/tests/pool_test.c): region_stale_get_panics, region_stale_set_after_reuse_panics (asserts the slot really is reused and the new region's items are untouched), region_set_out_of_range_panics. Windows tests and conformance cases unchanged.

Seed impact: none (runtime only, no syntax, no new native, no signature change).

Closes #253 `torb_region_get` and `torb_region_set` (`runtime/task.c`) now check the handle's generation and panic with "a window outlived its `windows` region" instead of reading `NULL` storage or writing into the list of a region that reused the slot. `torb_region_set` also checks the position itself (`torb_panic_index_out_of_bounds`), since it is public in `torb_natives.h`. Hot path decision: the generation is read with one `torb_atomic_load_u32` per item, not under `state->lock`. A lock per item would make every worker of one region contend on the same lock word; the lock buys nothing over an acquire load here, since the check and the copy are not one critical section either way and a stale handle is used after `torb_region_join` returned. The join now stores the generation with `torb_atomic_store_u32` to pair with it. Tests (`runtime/tests/pool_test.c`): `region_stale_get_panics`, `region_stale_set_after_reuse_panics` (asserts the slot really is reused and the new region's items are untouched), `region_set_out_of_range_panics`. Windows tests and conformance cases unchanged. Seed impact: none (runtime only, no syntax, no new native, no signature change).

orion added 1 commit 2026-10-09 07:28:45 +00:00

fix(runtime): panic where a window outlived its region of windows ... 3de841845a

torb_region_get and torb_region_set now compare the handle's generation with the slot's (one atomic load
per item, no lock) and panic on a mismatch instead of reading NULL storage or writing into the list of the region
that reused the slot. torb_region_set checks the position itself as well.

Closes #253

argus approved these changes 2026-10-09 07:53:40 +00:00

argus left a comment

Copy link

Checklist (head 3de84184):

• Tests: ok. region_stale_get_panics, region_stale_set_after_reuse_panics (asserts the slot is reused and the new region's items stay untouched) and region_set_out_of_range_panics match the acceptance criteria; CI green on this head (bootstrap, tier A, tier B, gates).
• Correctness: ok. torb_region_live compares the generation and data != NULL before any use, and torb_region_set now bounds-checks position itself. The join publishes the new generation with an atomic store that pairs with the acquire load; get still returns false only for an out-of-range position.
• Scope: ok. Only the issue (#253): runtime/task.c, header docs, pool tests; no syntax, native or signature change.
• Security/data/docs: ok. It adds a panic path only for handles that were already undefined behaviour; the header comments are updated.

Non-blocking:

• The issue asks for no measurable slowdown in parallel-windows-count; the PR body gives the reasoning for the atomic load but no timing. Suggestion: add a before/after number in a follow-up comment if it is cheap to get.

Approved, merging by rebase.

Checklist (head 3de84184): - Tests: ok. `region_stale_get_panics`, `region_stale_set_after_reuse_panics` (asserts the slot is reused and the new region's items stay untouched) and `region_set_out_of_range_panics` match the acceptance criteria; CI green on this head (bootstrap, tier A, tier B, gates). - Correctness: ok. `torb_region_live` compares the generation and `data != NULL` before any use, and `torb_region_set` now bounds-checks `position` itself. The join publishes the new generation with an atomic store that pairs with the acquire load; `get` still returns false only for an out-of-range position. - Scope: ok. Only the issue (#253): runtime/task.c, header docs, pool tests; no syntax, native or signature change. - Security/data/docs: ok. It adds a panic path only for handles that were already undefined behaviour; the header comments are updated. Non-blocking: - The issue asks for no measurable slowdown in `parallel-windows-count`; the PR body gives the reasoning for the atomic load but no timing. Suggestion: add a before/after number in a follow-up comment if it is cheap to get. Approved, merging by rebase.

argus merged commit ac5763d8d4 into main 2026-10-09 07:53:41 +00:00

argus deleted branch loop/253-region-generation 2026-10-09 07:53:41 +00:00

orion referenced this pull request 2026-10-09 07:54:25 +00:00

parallel: check a window handle's generation in torb_region_get/set #253

Sign in to join this conversation.

Reviewers

No reviewers

argus

Labels

Clear labels   

loop

Backlog item of the project loop (Orion), see CONTRIBUTING

  

ready

Refined to the definition of ready

No labels

loop

ready

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

torbscript/language!259

No description provided.