Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

portable-atomic-util: Rewrite Arc based on std::sync::Arc #140

Closed
taiki-e opened this issue Dec 17, 2023 · 3 comments · Fixed by #142
Closed

portable-atomic-util: Rewrite Arc based on std::sync::Arc #140

taiki-e opened this issue Dec 17, 2023 · 3 comments · Fixed by #142
Assignees
Labels
A-portable-atomic-util Area: related to portable-atomic-util crate

Comments

@taiki-e
Copy link
Owner

taiki-e commented Dec 17, 2023

When I reviewed #41, I thought the differences in its implementation were fine, but in reality, we have missed several problems that would not have occurred if the implementation had been based on std::sync::Arc (#137, #139, 1).

This will not necessarily be easy, since the standard library uses many unstable features.

Footnotes

  1. UPDATE: There are also cases where there are implementations that do not exist in the std::sync::Arc...

@taiki-e taiki-e added the A-portable-atomic-util Area: related to portable-atomic-util crate label Dec 17, 2023
@taiki-e taiki-e self-assigned this Dec 17, 2023
@notgull
Copy link
Contributor

notgull commented Dec 17, 2023

When I was originally writing the Arc code, I tried to follow along with the standard library's version, but polyfilling bits and pieces in when unstable features were used.

It might be nice to bump the MSRV of portable-atomic-util higher than 1.34 so we gain access to some of these features, such as addr_of.

@taiki-e
Copy link
Owner Author

taiki-e commented Dec 18, 2023

The implementation is in #142. addr_of_mut in as_ptr is not really important; the layout calculation is needed anyway to implement from_raw, and it can be used in as_ptr, so making the layout calculation work in the older version is the important point.

@taiki-e
Copy link
Owner Author

taiki-e commented Dec 23, 2023

As a side effect of this work, I found potential UBs in std Arc/Rc.
rust-lang/rust#119241

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-portable-atomic-util Area: related to portable-atomic-util crate
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants