refactor: fix Miri violations and improve concurrency safety

[SF-Zhou]

• Replace Box with AliasableBox to ensure pointer stability.
• Use AtomicU32 for refcnt and UnsafeCell for value storage in State.
• Store raw pointers in Entry guards instead of &mut references to avoid Stacked Borrows violations during concurrent atomic operations.
• Implement Sync for State<V> to explicitly define thread safety.
• Update README.md to reflect that Miri tests now pass.
• Optimize test constants for faster Miri execution.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (6c1fa5f) to head (d8cc7fc).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff            @@
##              main       #22   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            3         3           
  Lines          870       892   +22     
=========================================
+ Hits           870       892   +22     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Copilot]

Pull request overview

This PR refactors the internal concurrency implementation to fix Miri violations related to Stacked Borrows and improve thread safety guarantees. The changes replace Box with AliasableBox for pointer stability, use AtomicU32 for reference counting, wrap values in UnsafeCell, and store raw pointers instead of mutable references in entry guards.

• Replaced Box<State<V>> with AliasableBox<State<V>> to ensure heap stability when obtaining raw pointers
• Changed State to use AtomicU32 for refcnt and UnsafeCell for value storage to properly express interior mutability
• Modified entry guards (EntryByVal, EntryByRef) to store raw pointers instead of mutable references, avoiding Stacked Borrows violations during concurrent atomic operations
• Added Miri testing to CI and optimized test constants for faster Miri execution

Reviewed changes

Copilot reviewed 4 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/lockmap.rs	Core refactoring: replaced Box with AliasableBox, added AtomicU32/UnsafeCell for State, changed entry guards to use raw pointers, added explicit Sync implementations, and optimized test constants for Miri
src/shards_map.rs	Removed trailing whitespace from documentation
src/futex.rs	Removed trailing whitespace from documentation
README.md	Removed FAQ section explaining Miri violations since they are now fixed
Cargo.toml	Added aliasable dependency version 0.1.3
.github/workflows/rust.yml	Added new Miri CI job to run tests under Miri

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 6 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[purplesyringa]

May I ask why you're using a custom Mutex? It's a non-trivial piece of code, and you're basically simulating std's Mutex<Option<V>> with a custom Mutex + UnsafeCell<Option<V>>. Since you copied the code from std as per the comment in futex.rs, is this actually useful? How could you be achieving better performance than std?

[SF-Zhou]

May I ask why you're using a custom Mutex? It's a non-trivial piece of code, and you're basically simulating std's Mutex<Option<V>> with a custom Mutex + UnsafeCell<Option<V>>. Since you copied the code from std as per the comment in futex.rs, is this actually useful? How could you be achieving better performance than std?

What I actually need is std::sys::sync::Mutex, but it isn't publicly accessible. It is different from std::sync::Mutex, so I copied it from the standard library. I don't expect better performance, but its memory footprint is indeed smaller.

[purplesyringa]

Ah, due to poisoning I assume? That makes sense. I think std::sync::nonpoison::Mutex is what you actually need, since you have a mutex + some data that can only be safely accessed while that mutex is held. It's not yet stable, though, but hopefully it lands soon.

I was also wondering why you made refcnt atomic. As far as I can see, it's only updated from within various *update* callbacks, which give out a unique &mut reference to State, so atomics are seemingly unnecessary. You're also not using orderings except for relaxed, so it's just confusing with seemingly no benefit.

In general, I was wondering what the purpose of fn unlock is: you only invoke it from the destructor of EntryByVal, at which point state is already known and resolved. Can't it be inlined?

[SF-Zhou]

I was also wondering why you made refcnt atomic. As far as I can see, it's only updated from within various *update* callbacks, which give out a unique &mut reference to State, so atomics are seemingly unnecessary. You're also not using orderings except for relaxed, so it's just confusing with seemingly no benefit.

Under the checking rules of cargo miri, I am not actually allowed to obtain &mut State<V>, and thus I cannot get &mut refcnt. Therefore, I can only use AtomicU32 lol. Fortunately, I know that refcnt will only be accessed by one thread, so relaxed is a reasonable choice.

In general, I was wondering what the purpose of fn unlock is: you only invoke it from the destructor of EntryByVal, at which point state is already known and resolved. Can't it be inlined?

As for the fn unlock() you mentioned, both EntryByRef and EntryByVal need to call this function. Moreover, after mutex.unlock() is executed, other threads may have already acquired and updated the state. The modification of refcnt and the cleanup of state can only be safely done under the protection of the lock via self.map.

[purplesyringa]

Oh, I see now. The callbacks receive &mut AliasableBox<State<V>>, but other threads may hold *mut State<V> e.g. inside EntryByRef, so you cannot actually modify State<V>. You only modify refcnt under the global lock, so it doesn't really have to be atomic, and you could actually use u32 here just fine, but it avoids some unsafe, so that makes sense. Sorry for misunderstanding!

As for the fn unlock() you mentioned, both EntryByRef and EntryByVal need to call this function.

Ah, I didn't realize there are two consumers.

Moreover, after mutex.unlock() is executed, other threads may have already acquired and updated the state.

Hmm, isn't this a race condition? If thread A unlocks the per-element mutex first, and then thread B grabs it and does something with the element, and then A calls LockMap::unlock to e.g. commit the removal of the entry, it can possibly overwrite the value stored by B -- even though B saw the value stored by A and thus should be sequenced later.

That is, shouldn't the calls to global unlock and local unlock be swapped (not literally, since that will cause dangling pointers during removal, but at least metaphorically) so that the entry is modified first and then made accessible to other threads?

I think this is only ever a problem during removal, so the idea is something like "if the entry should remain present, only decrement refcount + unlock the entry; if the entry should be absent, only remove it from the map under a global lock".

[SF-Zhou]

I think this is only ever a problem during removal, so the idea is something like "if the entry should remain present, only decrement refcount + unlock the entry; if the entry should be absent, only remove it from the map under a global lock".

That makes sense, I'll give it a try.

Done in #24.