Skip to content

feat: support type erasure of pooled handles to unsized types - #585

Merged
Martin Taillefer (geeknoid) merged 5 commits into
mainfrom
plurality
Jul 22, 2026
Merged

Martin Taillefer (geeknoid) merged 5 commits into
mainfrom
plurality

Conversation

@geeknoid

Copy link
Copy Markdown
Member
  • Onboard bytesbuf and layered to plurality

@github-actions

github-actions Bot commented Jul 17, 2026 •

Copy link
Copy Markdown

✅ Version increments look sufficient

cargo semver-checks compared the 1 crate(s) this PR publishes against their previous version-bump commit in git history. Every version increment is sufficient for the detected API changes.

Crate Baseline Baseline commit This PR Minimum required Status
plurality 0.1.0 b18c99c 0.1.1 0.1.1 ✅ ok

This check is informational and does not block the merge.

View the check run

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends the plurality pooled-handle model to support type erasure to unsized types (dyn Trait and slices) for owning/shared handles, and migrates consumers (notably bytesbuf and layered) away from infinity_pool to plurality.

Changes:

  • Add DST/type-erasure support for plurality::Box, plurality::Arc, and plurality::Rc via unsize::Coercion, including erased reclaim logic in the pool.
  • Expand correctness + fuzz/property + allocation-tracking tests, plus new benchmark coverage for fat-pointer owning handles.
  • Migrate downstream crates to use plurality instead of infinity_pool, and update supporting docs/scripts/CI helpers.

Reviewed changes

Copilot reviewed 40 out of 42 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
justfiles/extended.just Ensure Loom runs with the loom feature enabled when executing loom_*.rs tests.
crates/plurality/tests/unsize.rs New integration tests covering Box/Arc/Rc unsizing to trait objects/slices and drop/reclaim behavior.
crates/plurality/tests/smart_ptr.rs Extend smart-pointer behavior assertions (ordering/hash set semantics).
crates/plurality/tests/pool.rs Add regression test coverage for panicking destructors returning slots.
crates/plurality/tests/loom_pool.rs Update Loom test invocation docs to match new test target/feature usage.
crates/plurality/tests/box.rs Add into_raw/from_raw round-trip tests for pooled Box.
crates/plurality/tests/bolero_pool.rs New Bolero property/fuzz test for core pool invariants.
crates/plurality/tests/alloc.rs Add regression test for panicking destructors returning local slots.
crates/plurality/tests/alloc_tracking.rs Add allocation-tracking assertions for the new dyn-box benchmark bodies.
crates/plurality/src/sync.rs Make Arc<T> support T: ?Sized and add Arc::unsize; use erased drop/reclaim.
crates/plurality/src/slot.rs Add MAX_POOL_SLOTS cap helper and clarify refcount/free-link semantics.
crates/plurality/src/rc.rs Make Rc<T> support T: ?Sized and add Rc::unsize; refcount via erased offsets.
crates/plurality/src/pool.rs Introduce PoolCore for erased teardown; implement erased reclaim from value ptr + size/align.
crates/plurality/src/lib.rs Document type erasure and re-export unsize::Coercion from the crate API.
crates/plurality/src/common.rs Add shared impl macros for T: ?Sized handles and update handle-layout docs.
crates/plurality/src/chunk.rs Remove type parameters from ChunkHeader and related layout/addressing helpers.
crates/plurality/src/builder.rs Build PoolCore with a concrete teardown callback; validate caps via MAX_POOL_SLOTS.
crates/plurality/src/boxed.rs Make Box<T> support T: ?Sized, add Box::unsize + into_raw/from_raw, and erased drop path.
crates/plurality/src/alloced.rs Update Alloc drop path to new local-free API shape.
crates/plurality/scripts/perf_report.rs Extend PERF report generation to include dyn-box benchmarks and ratio formatting.
crates/plurality/scripts/correctness.rs Update correctness harness to new Loom/Bolero test target names and flags.
crates/plurality/README.md Regenerate crate README with new type-erasure API docs and links.
crates/plurality/docs/PERF.md Update published performance report with new operations + dyn-box comparison section.
crates/plurality/docs/DESIGN.md Update design doc to reflect erased handles, PoolCore, and new recovery strategy.
crates/plurality/Cargo.toml Bump version; add unsize dep; add dyn-box benches; rename loom test target; move dev-deps.
crates/plurality/benches/shared/ops.rs Add Marker trait and unsize operations used by alloc benchmarks.
crates/plurality/benches/shared/dyn_box_ops.rs New shared bodies for fat-pointer owning-handle cross-crate comparison.
crates/plurality/benches/gungraun_dyn_box/main.rs New Callgrind bench harness for dyn-box comparison.
crates/plurality/benches/gungraun_dyn_box/linux.rs Linux implementation of dyn-box Callgrind benches.
crates/plurality/benches/gungraun_alloc/linux.rs Add box_unsize/arc_unsize to alloc Callgrind suite.
crates/plurality/benches/criterion_dyn_box.rs New wall-clock criterion benches for dyn-box comparison.
crates/plurality/benches/criterion_alloc.rs Add box_unsize/arc_unsize to criterion allocation suite.
crates/layered/src/dynamic.rs Switch dynamic-service implementation from infinity_pool to plurality with boxed DST handles.
crates/layered/Cargo.toml Replace infinity_pool feature/deps with optional plurality.
crates/fetch/Cargo.toml Formatting-only update to native-tls feature list.
crates/data_privacy_macros_impl/Cargo.toml Formatting-only update to allowed external types list.
crates/bytesbuf/src/mem/global.rs Replace raw infinity_pool block pooling with plurality, using raw-handle escape for self-handle.
crates/bytesbuf/Cargo.toml Replace infinity_pool dependency with plurality.
crates/bytesbuf_io/Cargo.toml Formatting-only update to categories.
Cargo.toml Bump plurality workspace dependency version and add unsize workspace dependency.
Cargo.lock Lockfile updates for plurality 0.1.1 and new unsize dependency usage.
.spelling Add unsize/unsizing to spelling allowlist.

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

Comment thread crates/plurality/src/rc.rs Outdated
Comment thread crates/plurality/tests/unsize.rs
Comment thread crates/bytesbuf/src/mem/global.rs Outdated
@codecov

codecov Bot commented Jul 17, 2026 •

Copy link
Copy Markdown

Codecov Report

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

Additional details and impacted files
@@           Coverage Diff            @@
##             main     #585    +/-   ##
========================================
  Coverage   100.0%   100.0%            
========================================
  Files         359      360     +1     
  Lines       27800    28002   +202     
========================================
+ Hits        27800    28002   +202     
Flag Coverage Δ
linux 94.0% <100.0%> (+8.2%) ⬆️
linux-arm 93.5% <100.0%> (+6.8%) ⬆️
windows 94.5% <100.0%> (+7.3%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread crates/plurality/tests/unsize.rs

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread crates/bytesbuf/src/mem/global.rs
Comment thread crates/plurality/benches/gungraun_dyn_box/linux.rs Outdated
Comment thread crates/plurality/docs/DESIGN.md Outdated
Comment thread crates/plurality/src/lib.rs Outdated
Comment thread crates/plurality/src/lib.rs Outdated
Comment thread crates/plurality/src/lib.rs
Comment thread crates/plurality/src/pool.rs
Comment thread crates/layered/src/dynamic.rs Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 40 out of 42 changed files in this pull request and generated 2 comments.

Comment thread crates/bytesbuf/src/mem/global.rs Outdated
Comment thread crates/plurality/src/pool.rs Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 41 out of 43 changed files in this pull request and generated 2 comments.

Comment thread crates/plurality/src/pool.rs
Comment thread crates/plurality/src/chunk.rs
Comment thread crates/plurality/Cargo.toml Outdated
@ralfbiedert

Copy link
Copy Markdown
Collaborator

See for example this here The Pin unsoundness and CoerceUnsized

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolve possible soundness questions

@geeknoid

Copy link
Copy Markdown
Member Author

Ralf Biedert (@ralfbiedert) Good call — rather than feature-gate it, I removed the unsize dependency entirely and vendored the small piece we need into plurality::coerce (one module). It no longer appears anywhere in the public API, and Cargo.lock/workspace deps drop it too.

The vendored version is also stronger than the original with respect to the soundness history you linked:

  • No fat-pointer layout assumptions / no byte-write. plurality only ever coerces its own slot: NonNull<T>. unsize() now just applies the compiler-generated unsizing closure directly — NonNull::new_unchecked((coercion.coerce)(ptr.as_ptr()).cast_mut()). That closure has body { ptr }, i.e. a genuine T -> U unsizing coercion checked by the compiler. So the fat-pointer metadata is compiler-produced (never forged), and we make zero assumptions about fat-pointer internals — we dropped the unsize crate’s set_ptr_value byte-write entirely.

  • The Pin + CoerceUnsized unsoundness class does not apply here. That issue requires coercing a Pin<P> while a malicious Deref/DerefMut observes the projection. plurality never coerces Pin<…> — it coerces a raw NonNull<T> pointing at a fixed-address pooled value, and there is no user-overridable Deref on the coercion path. The metadata comes from a real coercion, not from reconstructing a Pin.

  • Coercion is still re-exported (plurality::Coercion struct + Coercion! macro), but it is now backed by first-party code, so there is no unmaintained-crate exposure to reason about.

Happy to walk through crates/plurality/src/coerce.rs if useful — it is intentionally tiny.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Copilot AI review requested due to automatic review settings July 20, 2026 20:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 43 out of 45 changed files in this pull request and generated 2 comments.

Comment thread crates/plurality/src/pool.rs Outdated
Comment thread crates/plurality/src/pool.rs
@github-actions

github-actions Bot commented Jul 20, 2026 •

Copy link
Copy Markdown

⚠️ Potential breaking changes detected

cargo semver-checks flagged the following on this PR. This is informational -- breaking changes between commits are expected; the major-version bump happens at release time, not on every PR.

bytesbuf

    Building bytesbuf v0.6.0 (current)
       Built [   4.266s] (current)
     Parsing bytesbuf v0.6.0 (current)
      Parsed [   0.005s] (current)
    Building bytesbuf v0.6.0 (baseline)
       Built [   4.223s] (baseline)
     Parsing bytesbuf v0.6.0 (baseline)
      Parsed [   0.004s] (baseline)
    Checking bytesbuf v0.6.0 -> v0.6.0 (no change; assume minor)
     Checked [   0.022s] 196 checks: 195 pass, 1 fail, 0 warn, 49 skip

--- failure inherent_method_missing: pub method removed or renamed ---

Description:
A publicly-visible method or associated fn is no longer available under its prior name. It 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.46.0/src/lints/inherent_method_missing.ron

Failed in:
  BytesView::get_num_le, previously in file /home/runner/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/bytesbuf-0.6.0/src/view_get.rs:232
  BytesView::get_num_be, previously in file /home/runner/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/bytesbuf-0.6.0/src/view_get.rs:319
  BytesView::get_num_ne, previously in file /home/runner/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/bytesbuf-0.6.0/src/view_get.rs:409
  BytesBuf::put_num_le, previously in file /home/runner/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/bytesbuf-0.6.0/src/buf_put.rs:211
  BytesBuf::put_num_be, previously in file /home/runner/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/bytesbuf-0.6.0/src/buf_put.rs:238
  BytesBuf::put_num_ne, previously in file /home/runner/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/bytesbuf-0.6.0/src/buf_put.rs:266

     Summary semver requires new major version: 1 major and 0 minor checks failed
    Finished [   9.318s] bytesbuf

fetch

    Building fetch v0.13.0 (current)
       Built [  35.355s] (current)
     Parsing fetch v0.13.0 (current)
      Parsed [   0.007s] (current)
    Building fetch v0.13.0 (baseline)
       Built [  34.561s] (baseline)
     Parsing fetch v0.13.0 (baseline)
      Parsed [   0.007s] (baseline)
    Checking fetch v0.13.0 -> v0.13.0 (no change; assume minor)
     Checked [   0.019s] 196 checks: 194 pass, 2 fail, 0 warn, 49 skip

--- failure method_parameter_count_changed: pub method parameter count changed ---

Description:
A publicly-visible method now takes a different number of parameters, not counting the receiver (self) parameter.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#fn-change-arity
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.46.0/src/lints/method_parameter_count_changed.ron

Failed in:
  fetch::handlers::Metrics::layer now takes 0 parameters instead of 1, in /home/runner/work/oxidizer/oxidizer/crates/fetch/src/handlers/metrics.rs:235
  fetch::handlers::Logging::layer now takes 0 parameters instead of 2, in /home/runner/work/oxidizer/oxidizer/crates/fetch/src/handlers/logging.rs:43

--- failure method_requires_different_generic_type_params: method now requires a different number of generic type parameters ---

Description:
A method now requires a different number of generic type parameters than it used to. Uses of this method that supplied the previous number of generic types will be broken.
        ref: https://doc.rust-lang.org/reference/items/generics.html
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.46.0/src/lints/method_requires_different_generic_type_params.ron

Failed in:
  fetch::HttpClientBuilder::meter_provider takes 1 generic types instead of 0, in /home/runner/work/oxidizer/oxidizer/crates/fetch/src/client_builder.rs:254

     Summary semver requires new major version: 2 major and 0 minor checks failed
    Finished [  71.601s] fetch

Copilot AI review requested due to automatic review settings July 20, 2026 21:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Copilot AI review requested due to automatic review settings July 21, 2026 05:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread crates/plurality/README.md Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

@ralfbiedert

Ralf Biedert (ralfbiedert) commented Jul 21, 2026 •

Copy link
Copy Markdown
Collaborator

Ralf Biedert (@ralfbiedert) Good call — rather than feature-gate it, I removed the unsize dependency entirely and vendored the small piece we need into plurality::coerce (one module). It no longer appears anywhere in the public API, and Cargo.lock/workspace deps drop it too.

Could you-as-a-human please also look at and double check this? I can produce UB:

use core::marker::PhantomPinned;
use core::pin::Pin;
use core::sync::atomic::{AtomicPtr, Ordering};
use core::{mem, ptr};

use plurality::{Arc, Pool, coerce};

trait AddressSensitive {
    fn initialize(self: Pin<&Self>);
}

struct Value {
    self_ptr: AtomicPtr<Value>,
    byte: u8,
    _pin: PhantomPinned,
}

impl Value {
    fn new() -> Self {
        Self {
            self_ptr: AtomicPtr::new(ptr::null_mut()),
            byte: 42,
            _pin: PhantomPinned,
        }
    }

    /// This is "safe" because a value initialized through `Pin<&Self>` normally
    /// remains at that address until it is dropped.
    fn read_pinned_byte(&self) -> u8 {
        let self_ptr = self.self_ptr.load(Ordering::Relaxed);
        assert!(!self_ptr.is_null());
        // SAFETY: `initialize` records this value's pinned address.
        unsafe { ptr::read_volatile(ptr::addr_of!((*self_ptr).byte)) }
    }
}

impl AddressSensitive for Value {
    fn initialize(self: Pin<&Self>) {
        self.self_ptr.store(ptr::from_ref(self.get_ref()).cast_mut(), Ordering::Relaxed);
    }
}

fn main() {
    let pool = Pool::<Value>::new();
    let value = pool.alloc_arc(Value::new());
    let mut unpinned_alias = value.clone();

    let erased: Arc<dyn AddressSensitive> = Arc::unsize(value, coerce!(dyn AddressSensitive));
    let pinned = Arc::into_pin(erased);
    pinned.as_ref().initialize();
    drop(pinned);

    let moved = mem::replace(Arc::get_mut(&mut unpinned_alias).unwrap(), Value::new());
    drop(unpinned_alias);
    drop(pool);

    let _ = moved.read_pinned_byte();
}

This fails on miri with

error: Undefined Behavior: in-bounds pointer arithmetic failed: alloc393 has been freed, so this pointer is dangling
    --> crates\plurality\examples\pin_alias_move.rs:40:37
     |
  40 |         unsafe { ptr::read_volatile(ptr::addr_of!((*self_ptr).byte)) }
     |                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here

However, this is not directly related to Coerce, but a deeper flaw in the Arc::into_pin machinery introduced earlier, the pinning / coercion combo just brought it up the radar as a well-known minefield.

The Pin contract says

From the moment a value is pinned by constructing a Pinning pointer to it, that value must remain, valid, at that same address in memory, until its drop handler is called.

I wasn't able to make this fully safe, but I think the unsafe here is valid. Value::read_pinned_byte() only ever tries to access its self-referential pointer after it has been set, and it can only be set once pinned via a Pin<&Self>. Once that pin happens, Value must stay where it was no-matter-what. In a real world this can probably be triggered by some Future combinator / wrapper.

I think it would be prudent to remove at least the pinning API surface for now unless it has seen more human expert eyes.
I also think we are moving too fast where unsafe is involved in too short of a time to absorb them. This is the second time in a few weeks where we had LLMs falsely insist unsafe constructs are sound (including miri tests they created) in foundational abstractions. It would be unfortunate if in a few months we'd have to recall layers of APIs built on top of these if it turns out that some composition primitives didn't actually compose.

Martin Taillefer and others added 4 commits July 21, 2026 13:30
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 904658d0-c00a-4236-b9be-4feba06fe838
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 904658d0-c00a-4236-b9be-4feba06fe838
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 904658d0-c00a-4236-b9be-4feba06fe838

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread crates/layered/src/dynamic.rs Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 904658d0-c00a-4236-b9be-4feba06fe838

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

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.

5 participants