Skip to content

Remove large amounts of unsafe from Refs via 'detached' lock guards#323

Merged
xacrimon merged 6 commits into
xacrimon:masterfrom
conradludgate:rwlock-detached
Feb 1, 2025
Merged

Remove large amounts of unsafe from Refs via 'detached' lock guards#323
xacrimon merged 6 commits into
xacrimon:masterfrom
conradludgate:rwlock-detached

Conversation

@conradludgate

Copy link
Copy Markdown
Contributor

(stacked on top of #322)

I came up with this novel abstraction for reducing the dependencies on pointers within dashmap. We need to hold the lock guard for the shard.

Since Ref/Mut wants to get proper reference values eventually, it doesn't matter whether you convert pointers early or later. It seems to work all the same.

Thus, we can convert

pub struct Ref<'a, K, V> {
    _guard: RwLockReadGuard<'a, HashMap<K, V>>,
    k: *const K,
    v: *const V,
}

into

pub struct Ref<'a, K, V> {
    _guard: RwLockReadGuardDetached<'a>,
    k: &'a K,
    v: &'a V,
}

Since the detached guard does not own the data, it's inherently unsafe, but from my understanding and from asking others, it is still sound as long as the data is still coupled with the guard. Ref/Entry APIs still hold the guard, which upholds this invariant.

As a by-product of using this in mapref::multiple, I noticed that the RwLock guard is marked as GuardNotSend, meanwhile every iterator/ref is manually marked as Send/Sync. This seemed redundant, so I've additionally made the guard Send and then removed all manual send/sync impls.

@conradludgate conradludgate mentioned this pull request Feb 1, 2025

@xacrimon xacrimon left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

lgtm

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