feat: allocator-api and shared memory support

[bavshin-f5]

Not quite at the state I wanted to reach, but should be good to start the review.

Known issues and annoyances:

• NgxString size is 4 words (ptr, len, capacity, alloc). std::string::String is the same though, so this can be ignored for now.
• There's no non-deprecated Hasher impl in ::core. We may need to add another dependency or implement our own Hasher.
• The RwLock is just a tiniest bit evil. It supposed to work anywhere where nginx locks do, but I have doubts...
  I considered pthread_rwlock_t with pthread_rwlockattr_setpshared, but there's that one platform without POSIX threads.
• Some remaining casual unsoundness, esp. around the RbTree. We may consider making pure Rust implementations of the data structures with fallible allocation support as a separate crate in the future.
• Windows failure is due to postconfiguration not being invoked. Not reproducible locally with up-to-date toolchain.
• Still need to figure how to wrap ngx_shm_zone_t safely.

[Copilot]

Pull Request Overview

This PR adds allocator-api integration and shared memory support for NGINX modules by introducing new synchronization primitives, allocator-backed string and data structures, and wrappers around NGINX’s slab and pool APIs.

• Implement shared‐memory reader/writer locks (sync.rs)
• Add allocator‐aware NgxString, RbTree, SlabPool, and Pool types
• Integrate allocator-api2, update Cargo features, and add ngx_sched_yield shim

Reviewed Changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/sync.rs	RawSpinlock and RwLock for interprocess sync
src/lib.rs	Reexport allocator and sync modules
src/core/string.rs	NgxString with alloc support and tests
src/core/rbtree.rs	RbTree wrapper using nginx red‐black tree API
src/core/slab.rs	SlabPool allocator over ngx_slab_pool_t
src/core/pool.rs	Pool wrapper over ngx_pool_t with Allocator
src/core/mod.rs	Expose new rbtree and slab modules
src/allocator.rs	TryCloneIn trait and allocator helper functions
nginx-sys/src/lib.rs	Add ngx_sched_yield fallback implementation

Comments suppressed due to low confidence (3)

src/core/string.rs:580

• [nitpick] There is no test for the error path of append_within_capacity. Adding a case where the buffer is full would ensure correct overflow behavior is covered.

    fn test_string_comparisons() {

src/allocator.rs:10

• [nitpick] The TryCloneIn trait lacks doc comments on its purpose and usage. Consider adding a brief overview and example in the trait-level doc.

pub trait TryCloneIn: Sized {

nginx-sys/src/lib.rs:181

• The call to usleep isn’t imported or qualified, causing a compile error. Consider calling libc::usleep or importing usleep from the proper crate.

        usleep(1)

[fitzgen]

Hi! @pchickey asked me to look at the Allocator trait bits because I have some experience implementing and using that trait.

The dangling_aligned helper could probably be simplified a little bit if it didn't have a type parameter, and always returned a NonNull<u8>. The only caller I saw (unless I missed something) uses T = u8 and so if the type parameter was removed, then that also removes some of the questions around T's alignment versus the alignment dynamically passed in as an argument, makes some debug_assert!s unnecessary, etc.

The comments about overriding the provided default Allocator methods are really opportunistic and are things that would be nice if the underlying capability is there. (And if the underlying capability isn't there right now, you might want to consider whether it is worth adding in the future. See the comment about building up Vecs.)

But yeah, overall, looks good to me!

(Leaving this review as a "comment" because I don't want to presume myself to be an nginx expert who can authoritatively approve or request changes to PRs or anything like that; just sharing my thoughts after a once-over. Hopefully they are helpful!)

[xeioex]

Some minor issues I found up to "feat: owned byte string type with Allocator support"

[pchickey]

Really nice work. Handful of pretty minor nits.

[pchickey]

Please validate args len first

[bavshin-f5]

The len is validated by the config parser before invoking the callback. If it doesn't match the flags set in the ngx_command_t, it's totally fine to crash or fail in any other way.

I addressed this in a comment and added an assertion — likely redundant as Index ops are checked in debug.

[pchickey]

I'm not sure if this struct actually needs to be pub - it looks to me like its private to the implementation of all the other structs, but maybe I missed something. It looks like the two places this gets used its paired with a PhantomData<(K, V)> so maybe that aspect can be folded into the "Raw" iter?

The second line in the dod comment needs elaboration. What is the current item in the tree, the Item? What is the operation to remove an Item from the tree? Maybe the comment about validity to perform some specified remove operation actually belongs on the methods for RBTree::iter and RBTree::iter_mut, and on Iter and IterMut, if it matters to the consumers of those structs/methods.

[bavshin-f5]

After this comment I realized that I had no clear picture when writing ngx::core::rbtree, and thus failed in documenting the purpose.

The underlying tree is not quite a complete key-value map implementation. Sure, there is a key: ngx_rbtree_key_t (a.k.a. usize), and a value type which embeds the ngx_rbtree_node_t, but RbTree<K,V,A> type is not a safe wrapper for an arbitrary NGINX rbtree. It is an implementation of a Map<K,V> with ngx_rbtree_t as a building block.
RawIter though works on any ngx_rbtree_key_t and just accesses the tree nodes sorted by node.key, not being aware of the value type or allocation method.

In the end, I want to implement both a generic ngx_rbtree_t wrapper to work with references to the NGINX internal trees, and a high-level Map/Set/etc type for a pure Rust module code. I renamed things and added some comments to clarify that, but I don't believe I got the abstractions right yet.
I'll take some time to think about that while attending the conference.

The current implementation should be good enough for our internal projects though.

[pchickey]

I may be misunderstanding exactly what this private helper is used for (it should have a comment) but if the offset_of the node field is important, should this be #[repr(C)] to make sure the offset of node matches the C repr? It looks like the Layout of this gets passed to the allocator, do we need to guarantee that it has the same layout as the C repr?

[bavshin-f5]

This type has no C counterpart and will not be accessed from C. AFAIU, the field offsets are stable when used within the same Rust compilation target, notwithstanding the layout or repr, so we don't need to force a specific representation.

[bavshin-f5]

Appreciate the very detailed reviews.
Addressed most of the small nits with the last push, will take a pause to figure out how to improve the RbTree interface.