feat(bytesbuf)!: make MemoryShared thread-aware by sandersaares · Pull Request #539 · microsoft/oxidizer

sandersaares · 2026-07-01T05:55:48Z

Summary

Makes the bytesbuf MemoryShared abstraction thread-aware, closing a design gap where our thread-mobile (Send) memory providers were expected to be thread-aware but the abstraction did not require or express it. A provider now owns the decision of how and when to be thread-aware, and any adapter around it simply forwards relocation rather than imposing its own strategy.

Changes

MemoryShared now has ThreadAware as a supertrait, so the abstraction requires and expresses thread-awareness. This makes it possible to relocate erased/abstract memory providers correctly across threads.
MemoryShared gains an object-safe clone_boxed method, blanket-provided for any Clone provider, so a type-erased provider can be duplicated without knowing its concrete type. Every MemoryShared provider is therefore required to be Clone.
Renamed and redesigned CallbackMemory into WrappingMemory<T, FReserve>. It owns an inner: T where T: MemoryShared and forwards relocate to it. The reserve closure now receives the correctly-managed inner as an argument (Fn(&T, usize) -> BytesBuf) rather than owning thread-affine state itself; it may capture only inert configuration.
Reworked OpaqueMemory to own a Box<dyn MemoryShared>, forwarding both cloning (via clone_boxed) and ThreadAware relocation to the wrapped provider. It imposes no thread-awareness strategy of its own, leaving that decision entirely to the wrapped provider (e.g. GlobalPool, which already manages per-core state internally), and each clone is an independent provider.
Gave the test providers TransparentMemory and FixedBlockMemory no-op ThreadAware impls, since they hold no thread-affine mutable state.
Downstream: http_extensions drops its now-unnecessary Unaware wrapper around OpaqueMemory; bytesbuf_io test providers migrate to WrappingMemory.

Breaking changes

MemoryShared gained ThreadAware as a supertrait and a new clone_boxed method, so implementors must now also be ThreadAware (and Clone, which the blanket impl requires).
CallbackMemory is removed in favour of WrappingMemory.

Validation

just format, just readme, and just spellcheck all clean.
Tests pass for bytesbuf, bytesbuf_io, and http_extensions (unit + doctests), including new tests covering WrappingMemory/OpaqueMemory relocation forwarding and the no-op relocate on the test providers.
Clippy -D warnings clean; all remaining dependents build.
Mutation testing of the changed logic (cargo mutants --in-diff) leaves no surviving mutants.

Add `ThreadAware` as a supertrait to `MemoryShared` so the abstraction requires and expresses the thread-awareness our thread-mobile providers already guarantee. This makes it possible to relocate erased/abstract memory providers correctly across threads. Rename and redesign `CallbackMemory` into `WrappingMemory<T, FReserve>`, which owns an `inner: T` where `T: MemoryShared` and forwards `relocate` to it. The reserve closure now receives the correctly-managed `inner` as an argument (`Fn(&T, usize) -> BytesBuf`) rather than owning thread-affine state itself; it may capture only inert configuration. Rewrite `OpaqueMemory` over `thread_aware::Arc<dyn MemoryShared, PerCore>` so each core receives an independently relocatable clone; `OpaqueMemory::new` now requires `impl MemoryShared + Clone`. Give the test providers `TransparentMemory` and `FixedBlockMemory` no-op `ThreadAware` impls (they hold no thread-affine mutable state). Update downstream: `http_extensions` drops its now-unnecessary `Unaware` wrapper around `OpaqueMemory` and `with_custom_memory` gains a `+ Clone` bound; `bytesbuf_io` test providers migrate to `WrappingMemory`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-07-01T06:00:28Z

⚠️ Breaking Changes Detected


--- failure struct_missing: pub struct removed or renamed ---

Description:
A publicly-visible struct cannot be imported by its prior path. A `pub use` may have been removed, or the struct itself may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.48.0/src/lints/struct_missing.ron

Failed in:
  struct bytesbuf::mem::CallbackMemory, previously in file /home/runner/work/oxidizer/oxidizer/target/semver-checks/git-origin_main/e01cb86eb5dc66867eb31975dcb4fd53c16e5b41/crates/bytesbuf/src/mem/callback_memory.rs:46

--- failure trait_added_supertrait: non-sealed trait added new supertraits ---

Description:
A non-sealed trait added one or more supertraits, which breaks downstream implementations of the trait
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#generic-bounds-tighten
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.48.0/src/lints/trait_added_supertrait.ron

Failed in:
  trait bytesbuf::mem::MemoryShared gained ThreadAware in file /home/runner/work/oxidizer/oxidizer/crates/bytesbuf/src/mem/memory_shared.rs:22

--- failure trait_method_added: pub trait method added ---

Description:
A non-sealed public trait added a new method without a default implementation, which breaks downstream implementations of the trait
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#trait-new-item-no-default
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.48.0/src/lints/trait_method_added.ron

Failed in:
  trait method bytesbuf::mem::MemoryShared::clone_boxed in file /home/runner/work/oxidizer/oxidizer/crates/bytesbuf/src/mem/memory_shared.rs:28

If the breaking changes are intentional then everything is fine - this message is merely informative.

Remember to apply a version number bump with the correct severity when publishing a version with breaking changes (1.x.x -> 2.x.x or 0.1.x -> 0.2.x).

Copilot

Pull request overview

This PR makes bytesbuf’s MemoryShared abstraction explicitly thread-aware by requiring ThreadAware, ensuring providers can relocate thread-affine state when moved across threads. It also updates the type-erasure and wrapping story (OpaqueMemory, WrappingMemory) and migrates downstream/examples/tests accordingly.

Changes:

MemoryShared now requires ThreadAware; test providers add no-op ThreadAware impls.
Replaces CallbackMemory with WrappingMemory<T, FReserve> to wrap an inner MemoryShared and forward relocation correctly.
Reworks OpaqueMemory to use thread_aware::Arc<dyn MemoryShared, PerCore> (requiring Clone) and updates http_extensions + bytesbuf_io tests/examples.

Reviewed changes

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

Show a summary per file

File	Description
crates/http_extensions/src/body/builder.rs	Drops `Unaware` wrapper and requires `Clone` for custom memory to support thread-aware type erasure.
crates/bytesbuf/src/mem/wrapping_memory.rs	Adds `WrappingMemory` wrapper type (closure-based reservation + relocation forwarding) with tests.
crates/bytesbuf/src/mem/testing/transparent.rs	Adds no-op `ThreadAware` impl for `TransparentMemory`.
crates/bytesbuf/src/mem/testing/fixed_block.rs	Adds no-op `ThreadAware` impl for `FixedBlockMemory`.
crates/bytesbuf/src/mem/opaque_memory.rs	Reimplements `OpaqueMemory` using `thread_aware::Arc<..., PerCore>` and forwards relocation.
crates/bytesbuf/src/mem/mod.rs	Removes `CallbackMemory` module/export and exports `WrappingMemory`.
crates/bytesbuf/src/mem/memory_shared.rs	Makes `ThreadAware` a required supertrait of `MemoryShared` and documents the expectation.
crates/bytesbuf/src/mem/callback_memory.rs	Deletes the old `CallbackMemory` implementation.
crates/bytesbuf/src/lib.rs	Updates crate-level docs/doctest example to use `WrappingMemory`.
crates/bytesbuf/README.md	Regenerates README example to use `WrappingMemory`.
crates/bytesbuf/examples/bb_optimal_path.rs	Migrates example from `CallbackMemory` to `WrappingMemory` with thread-aware structure.
crates/bytesbuf/examples/bb_has_memory_optimizing.rs	Migrates example from `CallbackMemory` to `WrappingMemory` with thread-aware structure.
crates/bytesbuf_io/src/testing/pending.rs	Updates tests to use `WrappingMemory` (and inner provider forwarding) under `OpaqueMemory`.
crates/bytesbuf_io/src/testing/null.rs	Updates tests to use `WrappingMemory` (and inner provider forwarding) under `OpaqueMemory`.
crates/bytesbuf_io/src/testing/fake_write.rs	Updates tests to use `WrappingMemory` (and inner provider forwarding) under `OpaqueMemory`.
crates/bytesbuf_io/src/testing/fake_read.rs	Updates tests to use `WrappingMemory` (and inner provider forwarding) under `OpaqueMemory`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

codecov · 2026-07-01T06:09:18Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.0%. Comparing base (df08af3) to head (aa0d768).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #539   +/-   ##
=======================================
  Coverage   100.0%   100.0%           
=======================================
  Files         354      355    +1     
  Lines       26897    26908   +11     
=======================================
+ Hits        26897    26908   +11

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Redesign `OpaqueMemory` so it owns a `Box<dyn MemoryShared>` and forwards both cloning and `ThreadAware` relocation to the wrapped provider, rather than materializing per-core clones via `thread_aware::Arc<dyn MemoryShared, PerCore>`. The wrapped provider (e.g. `GlobalPool`, which already manages per-core state internally) now decides for itself how to be thread-aware, so the adapter imposes no `PerCore` strategy of its own and never double-layers thread-awareness. Express the cloneability requirement on the trait itself: `MemoryShared` gains an object-safe `clone_boxed` method, blanket-provided for any `Clone` provider, so type-erased providers can be duplicated without knowing their concrete type. `OpaqueMemory::new` and `HttpBodyBuilder::with_custom_memory` therefore no longer need a separate `Clone` bound. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Exercise the no-op `ThreadAware::relocate` on `TransparentMemory` and `FixedBlockMemory` so both are covered, verifying each provider stays usable after relocation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…losures Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

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

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.

ralfbiedert · 2026-07-01T09:18:19Z

 ///
 /// [thread-aware]: https://docs.rs/thread_aware
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, thread_aware::ThreadAware)]


Small nit, it's a bit weird this is used (or rather not use'ed) inconsistently.

ralfbiedert · 2026-07-01T09:22:59Z

+/// # Thread awareness
+///
+/// A memory provider shared across threads is expected to be [thread-aware][ThreadAware], so it can
+/// relocate any thread-affine state when moved between threads and avoid contention on


What does that mean for me as a user, like what is thread-affine state I should relocate? It couldn't be any memory handed out?

ralfbiedert · 2026-07-01T09:27:04Z

+/// ```
+///
+/// For a complete implementation pattern, see `examples/bb_has_memory_optimizing.rs`.
+pub struct WrappingMemory<T, FReserve>


WrappedMemory?

ralfbiedert · 2026-07-01T09:28:54Z

+/// # Inert closures
+///
+/// The closure must capture only inert configuration (e.g. flags, sizes) and never any thread-affine
+/// state such as another memory provider. Thread-affine state belongs in the wrapped `inner`


Would it be better to (also) say it should not capture any references or pointers? I find 'inert' configuration a bit of a weird term.

Alternatively, it could say it should only capture primitive or 'stack only' state?

ralfbiedert · 2026-07-01T09:31:12Z

+use crate::mem::{Memory, MemoryShared};
+
+/// Wraps an inner [`MemoryShared`] provider, customizing reservations via a closure.
+///


Also, why would I need this? I feel the example below is a bit made up.

sandersaares requested a review from Copilot July 1, 2026 06:03

Copilot started reviewing on behalf of sandersaares July 1, 2026 06:03 View session

Copilot AI reviewed Jul 1, 2026

View reviewed changes

Comment thread crates/bytesbuf/src/mem/opaque_memory.rs Outdated

sandersaares and others added 3 commits July 1, 2026 09:14

refactor(bytesbuf): prefer derive(ThreadAware) and simplify example c…

bf81419

…losures Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

sandersaares marked this pull request as ready for review July 1, 2026 07:28

Copilot AI review requested due to automatic review settings July 1, 2026 07:28

Copilot started reviewing on behalf of sandersaares July 1, 2026 07:28 View session

Copilot AI reviewed Jul 1, 2026

View reviewed changes

Comment thread crates/http_extensions/src/body/builder.rs Outdated

Potential fix for pull request finding

aa0d768

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings July 1, 2026 07:42

Copilot started reviewing on behalf of sandersaares July 1, 2026 07:42 View session

Copilot AI reviewed Jul 1, 2026

View reviewed changes

martintmk approved these changes Jul 1, 2026

View reviewed changes

ralfbiedert reviewed Jul 1, 2026

View reviewed changes

ralfbiedert approved these changes Jul 1, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(bytesbuf)!: make MemoryShared thread-aware#539

feat(bytesbuf)!: make MemoryShared thread-aware#539
sandersaares wants to merge 5 commits into
mainfrom
u/sasaares/memory-shared-thread-aware

sandersaares commented Jul 1, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 1, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

codecov Bot commented Jul 1, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

ralfbiedert Jul 1, 2026

Uh oh!

ralfbiedert Jul 1, 2026

Uh oh!

ralfbiedert Jul 1, 2026

Uh oh!

ralfbiedert Jul 1, 2026

Uh oh!

ralfbiedert Jul 1, 2026

Uh oh!

ralfbiedert Jul 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

sandersaares commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Breaking changes

Validation

Uh oh!

github-actions Bot commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ Breaking Changes Detected

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

codecov Bot commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

ralfbiedert Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

ralfbiedert Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

ralfbiedert Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

ralfbiedert Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

ralfbiedert Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

ralfbiedert Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sandersaares commented Jul 1, 2026 •

edited

Loading

github-actions Bot commented Jul 1, 2026 •

edited

Loading

codecov Bot commented Jul 1, 2026 •

edited

Loading