Skip to content

feat: add a safe wrapper for shared memory zones - #317

Draft
u5surf wants to merge 1 commit into
nginx:mainfrom
u5surf:feat/safe-shared-memory-zone
Draft

u5surf wants to merge 1 commit into
nginx:mainfrom
u5surf:feat/safe-shared-memory-zone

Conversation

@u5surf

@u5surf u5surf commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Proposed changes

Declaring a shared memory zone currently means calling ngx_shared_memory_add by hand, installing an unsafe extern "C" initialization callback, and casting the untyped ngx_slab_pool_t::data pointer back to the payload type at every access. Nothing ties those casts together, so a module using more than one zone can read a zone as the wrong type with no diagnostic.

This adds ngx::core::SharedZone<T>, which keeps the payload type in the signature:

struct SharedDict(RwLock<RbTreeMap<NgxString<SlabPool>, NgxString<SlabPool>, SlabPool>>);

impl SharedZoneData for SharedDict {
    fn new_in(alloc: SlabPool) -> Result<Self, AllocError> {
        Ok(Self(RwLock::new(RbTreeMap::try_new_in(alloc)?)))
    }
}

// directive handler
smcf.shm_zone = Some(SharedZone::add(cf, name, size, MyModule::module())?);

// request handling
let shared = smcf.shm_zone.as_ref().and_then(SharedZone::get)?;

SharedZone::add declares or joins a zone and installs a generic initialization callback; SharedZone::get hands back a &T without a cast. SharedZoneData::reuse is an optional hook for validating a value left behind by a previous configuration, which the raw interface offers no obvious place for.

Two design notes worth reviewing:

  • Zone contents are recovered from the slab pool rather than from ngx_shm_zone_t::data, so a zone reused across a configuration reload and a zone inherited from the master process on binary upgrade take the same path.
  • Zones declared noreuse are not covered: their mapping is replaced, and carrying data across that boundary needs the data argument this wrapper deliberately leaves alone. Happy to extend this if you would rather it be handled up front.

examples/shared_dict is ported over as the first user, which drops the ten unsafe blocks it needed for zone handling (57 → 48 unsafe in the file overall).

Marked as a draft: the API shape is the part I would like feedback on before polishing.

Testing

  • cargo test --lib, cargo clippy --all-targets, cargo fmt --check clean, with and without --features vendored.
  • examples/t/shared_dict.t against nginx 1.30.4 built from source: identical results before and after this change (3 pre-existing failures on macOS, unrelated to this PR — they concern updating an existing key in RbTreeMap, and reproduce on main).

Checklist

  • I have written my commit messages in the Conventional Commits format.
  • I have read the CONTRIBUTING doc
  • I have added tests (when possible) that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

Declaring a shared memory zone currently means calling
`ngx_shared_memory_add` by hand, installing an `unsafe extern "C"`
initialization callback, and casting the untyped
`ngx_slab_pool_t::data` pointer back to the payload type at every
access.  Nothing ties those casts together, so a module using more
than one zone can read a zone as the wrong type without any
diagnostic.

`SharedZone<T>` keeps the payload type in the signature instead.
`SharedZone::add` declares or joins a zone and installs a generic
initialization callback, and `SharedZone::get` hands back a `&T`
without a cast.  Implementers provide `SharedZoneData::new_in` to
build the initial value in the zone's slab pool, and may override
`SharedZoneData::reuse` to validate a value left behind by a previous
configuration, which the raw interface offers no obvious place for.

Zone contents are recovered from the slab pool rather than from
`ngx_shm_zone_t::data`, so a zone reused across a configuration reload
and a zone inherited from the master process on binary upgrade take
the same path.  Zones declared `noreuse` are not covered: their
mapping is replaced, and carrying data across that boundary needs the
`data` argument this wrapper deliberately leaves alone.

`examples/shared_dict` is ported over as the first user, dropping the
ten `unsafe` blocks it needed for zone handling.

Signed-off-by: Y.Horie <u5.horie@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant