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

Fix arc borrow provenance of, and add a test that actually uses ArcBorrow #78

Merged
merged 5 commits into from
Jan 9, 2024

Conversation

steffahn
Copy link
Contributor

@steffahn steffahn commented Jan 9, 2024

This PR also had to remove the ArcBorrow::from_ref API since it was unconditionally unsound to ever use.

Further breakage is in the former Send/Sync implementation, as this fixes #66

Fixes #65.

This PR supersedes #64. Sorry for the long delay 😅

@Manishearth
Copy link
Owner

Thanks! CI fails unfortunately.

@steffahn
Copy link
Contributor Author

steffahn commented Jan 9, 2024

I missed testing with --all-features :-)

Should work now…

@steffahn
Copy link
Contributor Author

steffahn commented Jan 9, 2024

remove the ArcBorrow::from_ref API since it was unconditionally unsound to ever use

Actually, maybe not all usages would be unsound, but still, creating new copies of the Arc from ArcBorrow::from_ref-created ArcBorrow was unsound, and that ability is the whole point of ArcBorrow, so yeah. As you can see, I went with a rename to from_ptr (instead of just changing from_ref to take a *const T argument) which should still be an easy to find method, it ensures users are becoming aware of the issue, and it documents the new safety requirements.

@Manishearth Manishearth merged commit 6018136 into Manishearth:master Jan 9, 2024
4 checks passed
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.

Unsound Send/Sync of ArcBorrow. UB in ArcBorrow implementation due to provenance
2 participants