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

Basic CTL shared_ptr implementation #1267

Merged
merged 13 commits into from
Aug 31, 2024
Merged

Conversation

mrdomino
Copy link
Collaborator

No description provided.

@mrdomino
Copy link
Collaborator Author

Supersedes #1229.

@mrdomino mrdomino requested a review from jart August 28, 2024 18:59
mrdomino and others added 7 commits August 28, 2024 14:38
Needed for the placement-new in shared_emplace to compile properly.
A shared_ptr<void> is not anonymous, it's just a wrapped void*.
Put a noexcept annotation on drop_shared (since dispose is also noexcept
for now) and simplify reset, fixing a bug in weak_ptr::swap.
Copy link
Owner

@jart jart left a comment

Choose a reason for hiding this comment

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

Looks great. We know this algorithm is correct, but if you want to make extra sure, you could add a pthread_create torture test. I have lots of them in the codebase.

__atomic_fetch_add(r, 1, __ATOMIC_RELAXED);
#else
size_t refs = __atomic_fetch_add(r, 1, __ATOMIC_RELAXED);
if (refs > ((size_t)-1) >> 1)
Copy link
Owner

Choose a reason for hiding this comment

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

Is there any particular reason why you wouldn't just use long and then check if it's negative?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You’d need ckd_add or something in that case - I believe signed integer overflow is UB

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh wait, no, I see what you mean. Yeah, that could work.

@mrdomino mrdomino merged commit e1528a7 into jart:master Aug 31, 2024
6 checks passed
@mrdomino mrdomino deleted the ctl-shared2 branch August 31, 2024 18:00
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.

2 participants