I feel like the bound might be unnecessary and thus overly restrictive. I'm writing this down as an issue first to allow others to look at this, too, (or for myself to take a second look) before contributing a change (removing the T: 'static line here).

Hello, great work on this project. My project, Ribir, depends on your project's master branch. I noticed there hasn't been a release yet. I'm wondering if there's a plan for a new release soon, as it would help my project to depend on a stable version. Thanks for considering.

ArcBorrow<'_, T> contains an actual &T reference. Such a reference cannot have provenance over the reference count though, which leads to miri detecting UB if you run cargo +nightly miri test on #64.

git fetch origin bf35d79324665bd4886a4319216a0964a958dda5
git checkout bf35d79324665bd4886a4319216a0964a958dda5
cargo +miri test

running 33 tests
test arc::tests::arc_eq_and_cmp ... ok
test arc::tests::arc_eq_and_partial_cmp ... ok
test arc::tests::from_iterator_exact_size ... ok
test arc::tests::from_iterator_unknown_size ... ok
test arc::tests::maybeuninit ... ok
test arc::tests::maybeuninit_array ... ok
test arc::tests::maybeuninit_slice_ub_to_proceed - should panic ... ok
test arc::tests::maybeuninit_ub_to_proceed - should panic ... ok
test arc::tests::roundtrip ... ok
test arc::tests::roundtrip_slice ... ok
test arc::tests::try_unwrap ... ok
test arc_borrow::clone_arc_borrow ... error: Undefined Behavior: trying to retag from <346713> for SharedReadWrite permission at alloc90475[0x0], but that tag does not exist in the borrow stack for this location
   --> src/arc.rs:230:18
    |
230 |         unsafe { &*self.ptr() }
    |                  ^^^^^^^^^^^^
    |                  |
    |                  trying to retag from <346713> for SharedReadWrite permission at alloc90475[0x0], but that tag does not exist in the borrow stack for this location
    |                  this error occurs as part of retag at alloc90475[0x0..0x10]
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
    = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: <346713> was created by a SharedReadOnly retag at offsets [0x8..0xc]
   --> src/arc_borrow.rs:39:42
    |
39  |         let arc = unsafe { Arc::from_raw(self.0) };
    |                                          ^^^^^^
    = note: BACKTRACE (of the first span):
    = note: inside `arc::Arc::<i32>::inner` at src/arc.rs:230:18: 230:30
note: inside `<arc::Arc<i32> as std::clone::Clone>::clone`
   --> src/arc.rs:423:24
    |
423 |         let old_size = self.inner().count.fetch_add(1, Relaxed);
    |                        ^^^^^^^^^^^^
note: inside `arc_borrow::ArcBorrow::<'_, i32>::clone_arc`
   --> src/arc_borrow.rs:41:21
    |
41  |         mem::forget(arc.clone());
    |                     ^^^^^^^^^^^
note: inside `arc_borrow::clone_arc_borrow`
   --> src/arc_borrow.rs:122:13
    |
122 |     let y = b.clone_arc();
    |             ^^^^^^^^^^^^^
note: inside closure
   --> src/arc_borrow.rs:119:22
    |
118 | #[test]
    | ------- in this procedural macro expansion
119 | fn clone_arc_borrow() {
    |                      ^
    = note: this error originates in the attribute macro `test` (in Nightly builds, run with -Z macro-backtrace for more info)

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to previous error

error: test failed, to rerun pass `--lib`

Caused by:
  process didn't exit successfully: `/home/frank/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/cargo-miri runner /home/frank/forks/just-a-clone/triomphe/target/miri/x86_64-unknown-linux-gnu/debug/deps/triomphe-70e77d022ca4bab7` (exit status: 1)

I should have time tomorrow to work on a fix, probably comparable to the implementation of rc_borrow::ArcBorrow which uses a raw (NonNull) pointer instead of a reference.

Test coverage here is pretty abysmal. Filing a bug to record that it should be improved.

The stdlib has some tests for Arc functionality we could borrow in https://github.com/rust-lang/rust/blob/master/src/liballoc/sync/tests.rs, but a lot of them test things we don't really support like weak references.

They also don't test any of our bespoke artisanal smart pointers.

It would also be nice to run tests under loom once we actually have tests.

... as nodrop is deprecated and ManuallyDrop is the zero-cost way of achieving its goal now.

^

Currently, the implicit

impl<'a, T: ?Sized> Send for ArcBorrow<'a, T>
where
    T: Sync,
impl<'a, T: ?Sized> Sync for ArcBorrow<'a, T>
where
    T: Sync,

don’t match the implementations for &Arc<T> . This is unsound, as T: Send + Sync (not just T: Sync) is essential for payloads dropped in a different thread than the one creating the Arc, as well as for APIs such as Arc::get_mut, etc…

For #65, ArcBorrow needs to be changed anyways, so I suppose this could be fixed at the same time. Of course, that’s then technically a breaking change, but also a soundness fix.

It’s like a Box, so it can support Send/Sync implementations like a Box.

Currently, it’s

impl<T: ?Sized> Send for UniqueArc<T>
where
    T: Send + Sync,
impl<T: ?Sized> Sync for UniqueArc<T>
where
    T: Send + Sync,

but as far as I can tell, it should be entirely sound to have it instead be

impl<T: ?Sized> Send for UniqueArc<T>
where
    T: Send,
impl<T: ?Sized> Sync for UniqueArc<T>
where
    T: Sync,

Specifically, the panic comes from the slice indexing at line 593 in lib.rs

The difficulty with copying these impls directly from std is that you need to have a version of from_raw that works with ?Sized types. This currently doesn't work because memoffset::offset_of only supports Sized types.

@Manishearth might have a way to do it using the existing support for DSTs.

• From<&[T]>
• From<&CStr>
• From<&OsStr>
• From<&Path>
• From<&str>
• From<Arc<W>>
• From<Box<T>>
• From<CString>
• From<OsString>
• From<PathBuf>
• From<String>
• From<Vec<T>>

write is defined like this:

impl<T> Arc<MaybeUninit<T>> {
    pub fn write(&mut self, val: T) -> &mut T {
        unsafe {
            let ptr = (*self.ptr()).data.as_mut_ptr();
            ptr.write(val);
            &mut *ptr
        }
    }
}

It writes to the inner value of the Arc even though unique access to Arc does not guarantee unique access to the inner value (b/c of clones). Here is a simple test case that fails under miri:

let mut uninit = Arc::new_uninit();
let clone = uninit.clone();

let x: &MaybeUninit<String> = &*clone;

// This write invalidates `x` reference
uninit.write(String::from("nonononono"));

// Read invalidated reference to trigger UB
let _ = &*x;

error: Undefined Behavior: trying to reborrow <205779> for SharedReadOnly permission at alloc89625[0x8], but that tag does not exist in the borrow stack for this location
   --> src/lib.rs:107:5
    |
107 |     &*_x;
    |     ^^^^
    |     |
    |     trying to reborrow <205779> for SharedReadOnly permission at alloc89625[0x8], but that tag does not exist in the borrow stack for this location
    |     this error occurs as part of a reborrow at alloc89625[0x8..0x20]
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
    = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: <205779> was created by a retag at offsets [0x8..0x20]
   --> src/lib.rs:103:36
    |
103 |     let _x: &MaybeUninit<String> = &*clone;
    |                                    ^^^^^^^
help: <205779> was later invalidated at offsets [0x8..0x20]
   --> src/arc.rs:247:23
    |
247 |             let ptr = (*self.ptr()).data.as_mut_ptr();
    |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    = note: inside `ub` at src/lib.rs:107:5
note: inside closure at src/lib.rs:97:1
   --> src/lib.rs:97:1
    |
96  |   #[test]
    |   ------- in this procedural macro expansion
97  | / fn ub() {
98  | |     use core::mem::MaybeUninit;
99  | |
100 | |     let mut uninit = Arc::new_uninit();
...   |
107 | |     &*_x;
108 | | }
    | |_^
    = note: this error originates in the attribute macro `test` (in Nightly builds, run with -Z macro-backtrace for more info)

Arc::into_raw does not require Sized, but Arc::from_raw does. Must be a mistake.

Edit: I missed todo

However, for now Arc can have an operation like Arc::from_raw_slice. Practically many unsized types are just slices.

I have a setup where I want to do

let ptr = thin_arc.with_arc(Arc::clone).into_raw();

// store the raw pointer somewhere. then later

let thin_arc = ThinArc::from_data_raw(ptr);

I would go through Arc::from_raw(ptr).into_thin() but that doesn't work because from_raw needs T: Sized.

What I'm imagining is in this impl there is something like

    /// Reconstruct the Arc<HeaderSliceWithLength<H, [T]>> from a raw pointer obtained from into_raw()
    #[inline]
    pub unsafe fn from_thin_raw(ptr: *const HeaderSliceWithLength<H, [T]>) -> ThinArc<H, T> {
        let data_ptr = ptr as *const [u8] as *const u8;
        let thin_ptr = data_ptr.sub(offset_of!(ArcInner<HeaderSliceWithLength<H, [T; 0]>>, data));
        ThinArc {
            ptr: unsafe {
                ptr::NonNull::new_unchecked(
                    thin_ptr as *mut ArcInner<HeaderSliceWithLength<H, [T; 0]>>,
                )
            },
            phantom: PhantomData,
        }
    }

For context, I was trying to move some of the functionality from https://github.com/conradludgate/futures-buffered/blob/main/src/arc_slice.rs into triomphe.

The core idea is that I have ThinArc<H, Slot> where each Slot stores it's index in the slice.
Given a *const Slot, I can count back the index and get back the original ThinArc<H, Slot>. Almost everything was working, except for the drop() impl of the RawWaker, since I couldn't construct the arc with the correct size to deallocate.

from_header_and_iter calls alloc::handle_alloc_error, which unfortunately is an unconditional process-killing abort.

I have a situation where real OOM really happens, and I really need to handle it gracefully.

Could you replace the call to handle_alloc_error with a regular panic!("oom"), at least as an option? A try_ version of the method would be fine too.

In my code I use ThinArc<(), u8> a lot, and apparently the compiler can't optimize the iterator in from_header_and_iter away. It really does iterate over each byte and writes it using ptr::write. I implemented an optimization that accepts a whole slice of a Copy type and uses ptr::copy_nonoverlapping instead, and I got a 5% speedup in my application. I can open a PR if there is interest.

struct SliceArc<T> {
  data: NonNull<T>,
  len: usize,
}

impl Deref for SliceArc<T> {
  type Target = &[T];

  fn deref(&self) -> &[T] {
    slice::from_raw_parts(self.data, self.len)
  }
}

When len == 0, it does no allocation, const-constructible, data is dangling pointer.

When len != 0, data holds a pointer to slice data, and arc headers lies in memory before the data.

Add methods that mirror std::sync::Arc::try_unwrap() that allow getting the inner value if the refcount is zero

Hi @Manishearth, any chance for a new release with the latest changes? Thanks!

std::sync::Arc implements FromIterator.

thiomphe::Arc should implement it too.

I searched the usage of memoffset.
https://github.com/Manishearth/triomphe/search?q=offset_of
I find that there are only three offset_of! and the layout of data structures are stable. So it's easy to replace the macro with simple constants.

A ThinArcStr would be pretty easy to support (would just wrap a ThinArc<H, u8> -- you can already do this by using triomphe).

The use case I care more about would be a way of having a 1-pointer Arc (pretty useful for FFI stuff)... but I'm not sure how possible this is though, given that we don't have a way to get a trait object vtable pointer...

I'm using triomphe for the cstree library, a fork of @matklad's rowan syntax tree library used in rust-analyzer. With the latest nightly, miri emits an error for potential undefined behaviour for the reborrow in Arc::inner.

The reborrow occurs while Arc::cloneing a pointer obtained from Arc::new. In the specific instance posted below, it is triggered by the sytax::text::tests::test_text_equality, but I've ran it against other tests and the error is very reproducible in any test that builds a syntax tree.

There were a few miri warnings on the cstree side related to the relatively recent strict provenance development, which I addressed, so only the reborrow remains.

test syntax::text::tests::test_text_equality ... error: Undefined Behavior: trying to reborrow <232138> for SharedReadWrite permission at alloc98393[0x0], but that tag does not exist in the borrow stack for this location
 --> /home/dquirl/.cargo/registry/src/github.com-1ecc6299db9ec823/triomphe-0.1.6/src/arc.rs:214:18
    |
214 |         unsafe { &*self.ptr() }
    |                  ^^^^^^^^^^^^
    |                  |
    |                  trying to reborrow <232138> for SharedReadWrite permission at alloc98393[0x0], but that tag does not exist in the borrow stack for this location
    |                  this error occurs as part of a reborrow at alloc98393[0x0..0x8]
    |
  = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
  = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: <232138> was created by a retag at offsets [0x8..0x14]
 --> src/green/token.rs:50:19
    |
50  |         let ptr = Arc::into_raw(Arc::new(data));
    |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  = note: inside `triomphe::Arc::<green::token::GreenTokenData>::inner` at /home/dquirl/.cargo/registry/src/github.com-1ecc6299db9ec823/triomphe-0.1.6/src/arc.rs:214:18
  = note: inside `<triomphe::Arc<green::token::GreenTokenData> as std::clone::Clone>::clone` at /home/dquirl/.cargo/registry/src/github.com-1ecc6299db9ec823/triomphe-0.1.6/src/arc.rs:322:24
note: inside `<green::token::GreenToken as std::clone::Clone>::clone` at src/green/token.rs:103:27
 --> src/green/token.rs:103:27
    |
103 |             Arc::into_raw(Arc::clone(&arc))
    |                           ^^^^^^^^^^^^^^^^
note: inside `green::builder::NodeCache::token` at src/green/builder.rs:216:9
 --> src/green/builder.rs:216:9
    |
216 | /         self.tokens
217 | |             .entry(data)
218 | |             .or_insert_with_key(|data| GreenToken::new(*data))
219 | |             .clone()
    | |____________________^
note: inside `green::builder::GreenNodeBuilder::token` at src/green/builder.rs:408:21
 --> src/green/builder.rs:408:21
    |
408 |         let token = self.cache.token(kind, text);
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `syntax::text::tests::build_tree` at src/syntax/text.rs:421:13
 --> src/syntax/text.rs:421:13
    |
421 |             builder.token(SyntaxKind(92), chunk);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `syntax::text::tests::test_text_equality::do_check` at src/syntax/text.rs:431:34
 --> src/syntax/text.rs:431:34
    |
431 |             let (t1, resolver) = build_tree(t1);
    |                                  ^^^^^^^^^^^^^^
note: inside `syntax::text::tests::test_text_equality::check` at src/syntax/text.rs:442:13
 --> src/syntax/text.rs:442:13
    |
442 |             do_check(t1, t2);
    |             ^^^^^^^^^^^^^^^^
note: inside `syntax::text::tests::test_text_equality` at src/syntax/text.rs:446:9
 --> src/syntax/text.rs:446:9
    |
446 |         check(&[""], &[""]);
    |         ^^^^^^^^^^^^^^^^^^^
note: inside closure at src/syntax/text.rs:429:5
 --> src/syntax/text.rs:429:5
    |
428 |       #[test]
    |       ------- in this procedural macro expansion
429 | /     fn test_text_equality() {
430 | |         fn do_check(t1: &[&str], t2: &[&str]) {
431 | |             let (t1, resolver) = build_tree(t1);
432 | |             let t1 = t1.resolve_text(&resolver);
...   |
456 | |         check(&["{", "abc", "}ab"], &["{", "abc", "}", "ab"]);
457 | |     }
    | |_____^

I've also ran miri again with -Zmiri-track-pointer-tag=232138, which curiously (to me, as a definite non-expert on miri) only shows where the tag is created (which is in Arc::into_raw when constructing the cstree::GreenToken::new) and no pop/tag invalidation (note that there is also no diagnostic note about invalidation in the error), so potentially this could also be Miri misfiring?

test syntax::text::tests::test_text_equality ... note: tracking was triggered
   --> /home/dquirl/.cargo/registry/src/github.com-1ecc6299db9ec823/triomphe-0.1.6/src/arc.rs:100:18
    |
100 |         unsafe { &((*self.ptr()).data) as *const _ }
    |                  ^^^^^^^^^^^^^^^^^^^^^ created tag 232138
    |
    = note: inside `triomphe::Arc::<green::token::GreenTokenData>::as_ptr` at /home/dquirl/.cargo/registry/src/github.com-1ecc6299db9ec823/triomphe-0.1.6/src/arc.rs:100:18
    = note: inside `triomphe::Arc::<green::token::GreenTokenData>::into_raw` at /home/dquirl/.cargo/registry/src/github.com-1ecc6299db9ec823/triomphe-0.1.6/src/arc.rs:90:19
note: inside `green::token::GreenToken::new` at src/green/token.rs:50:19
   --> src/green/token.rs:50:19
    |
50  |         let ptr = Arc::into_raw(Arc::new(data));
    |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside closure at src/green/builder.rs:218:40
   --> src/green/builder.rs:218:40
    |
218 |             .or_insert_with_key(|data| GreenToken::new(*data))
    |                                        ^^^^^^^^^^^^^^^^^^^^^^
    = note: inside `std::collections::hash_map::Entry::<green::token::GreenTokenData, green::token::GreenToken>::or_insert_with_key::<[closure@src/green/builder.rs:218:33: 218:62]>` at /home/dquirl/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/collections/hash/map.rs:2577:29
note: inside `green::builder::NodeCache::token` at src/green/builder.rs:216:9
   --> src/green/builder.rs:216:9
    |
216 | /         self.tokens
217 | |             .entry(data)
218 | |             .or_insert_with_key(|data| GreenToken::new(*data))
    | |______________________________________________________________^
note: inside `green::builder::GreenNodeBuilder::token` at src/green/builder.rs:408:21
   --> src/green/builder.rs:408:21
    |
408 |         let token = self.cache.token(kind, text);
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `syntax::text::tests::build_tree` at src/syntax/text.rs:421:13
   --> src/syntax/text.rs:421:13
    |
421 |             builder.token(SyntaxKind(92), chunk);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `syntax::text::tests::test_text_equality::do_check` at src/syntax/text.rs:431:34
   --> src/syntax/text.rs:431:34
    |
431 |             let (t1, resolver) = build_tree(t1);
    |                                  ^^^^^^^^^^^^^^
note: inside `syntax::text::tests::test_text_equality::check` at src/syntax/text.rs:442:13
   --> src/syntax/text.rs:442:13
    |
442 |             do_check(t1, t2);
    |             ^^^^^^^^^^^^^^^^
note: inside `syntax::text::tests::test_text_equality` at src/syntax/text.rs:446:9
   --> src/syntax/text.rs:446:9
    |
446 |         check(&[""], &[""]);
    |         ^^^^^^^^^^^^^^^^^^^
note: inside closure at src/syntax/text.rs:429:5
   --> src/syntax/text.rs:429:5
    |
428 |       #[test]
    |       ------- in this procedural macro expansion
429 | /     fn test_text_equality() {
430 | |         fn do_check(t1: &[&str], t2: &[&str]) {
431 | |             let (t1, resolver) = build_tree(t1);
432 | |             let t1 = t1.resolve_text(&resolver);
...   |
456 | |         check(&["{", "abc", "}ab"], &["{", "abc", "}", "ab"]);
457 | |     }
    | |_____^
    = note: this note originates in the attribute macro `test` (in Nightly builds, run with -Z macro-backtrace for more info)

Nightly version: rustc 1.64.0-nightly (7425fb293 2022-06-30).

Worked with rustc 1.63.0-nightly (76761db59 2022-05-24).

It would be nice to have a version of Rc without weak reference counts too.

• when byte_sub is stabilized, this can accept T: ?Sized.
  

  triomphe/src/arc.rs

  Line 82 in 1bce266

  // FIXME: when `byte_sub` is stabilized, this can accept T: ?Sized.

• (bholley): Use the updated comment when [2] is merged.
  

  triomphe/src/arc.rs

  Line 599 in 1bce266

  // FIXME(bholley): Use the updated comment when [2] is merged.

• should from_ref be relaxed to unsized types? It can't be
  

  triomphe/src/arc_borrow.rs

  Line 47 in 1bce266

  /// TODO: should from_ref be relaxed to unsized types? It can't be

• once pointer::with_metadata_of is stable or
  

  triomphe/src/header.rs

  Line 212 in 1bce266

  // FIXME: once `pointer::with_metadata_of` is stable or

Moving TODO-s to a GitHub issue, so contributors may see them.

I don't know if this is possible, but it would be helpful for my use case (as a heuristic to avoid an expensive operation, not as something to be relied on for correctness) to be able to check if the refcount of a reference is currently <= 2.

Hi! We depend on this library in domain transitively via moka and just got bit by new requirement that rust 1.76 or later should be used. This is fine of course, but I wanted to ask if you could specify some MSRV policy or provide the rust-version key in Cargo.toml, which will help prevent issues like this once the MSRV-dependent resolver lands.

Hi,

@michaelsproul and I ran some benchmarks in rpds . Most results improved when switching from std::sync::Arc to triomphe::Arc, but I was surprised that some benchmarks regressed significantly (> 30%). I'm wondering if there are optimizations on std::sync::Arc that were never ported to triomphe and that could improve triomphe and all projects that use it.

This is the PR with the benchmark results: orium/rpds#88

Something along these lines would add it, I think:

#![cfg_attr(
    feature = "unstable",
    feature(coerce_unsized, unsize, dispatch_from_dyn)
)]
#[cfg(feature = "unstable")]
use core::{
    marker::Unsize,
    ops::{CoerceUnsized, DispatchFromDyn},
};

#[cfg(feature = "unstable")]
impl<T: ?Sized + Unsize<U>, U: ?Sized> DispatchFromDyn<Arc<U>> for Arc<T> {}
#[cfg(feature = "unstable")]
impl<T: ?Sized + Unsize<U>, U: ?Sized> CoerceUnsized<Arc<U>> for Arc<T> {}
#[cfg(feature = "unstable")]
impl<T: ?Sized + Unsize<U>, U: ?Sized> CoerceUnsized<UniqueArc<U>> for UniqueArc<T> {}

My hesitation is that I don't fully understand DispatchFromDyn, nor do I understand all the implications of these traits. I also don't need it myself.