Skip to content

Add conversions from Arc<Mutex> and Arc<RwLock> wrappers#26

Merged
Luthaf merged 4 commits intomainfrom
lock-everything
Apr 9, 2026
Merged

Add conversions from Arc<Mutex> and Arc<RwLock> wrappers#26
Luthaf merged 4 commits intomainfrom
lock-everything

Conversation

@Luthaf
Copy link
Copy Markdown
Member

@Luthaf Luthaf commented Mar 16, 2026

This allows sharing data that can be modified both by Rust and the DlPack consumer, using locks to enforce borrowing rules at runtime.

We store both the Arc<Lock> and the LockGuard<'a> in the DLPack context to ensure that we both keep the data alive in the Arc, and hold the corresponding lock. The resulting self-referential struct (i.e. struct that borrows itself) is created through ouroboros.

@Luthaf Luthaf requested a review from HaoZeke March 16, 2026 10:45
Copy link
Copy Markdown
Member

@HaoZeke HaoZeke left a comment

Choose a reason for hiding this comment

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

TBH I don't like ouroboros. It's a lot of codegen for what I'd think should be maybe handled with simpler lifetime transmute? like

struct ManagerContextMutex<T: 'static> {
    // guard declared before _arc
    // since drops are in declaration order.
    guard: MutexGuard<'static, Vec<T>>,
    _arc: Arc<Mutex<Vec<T>>>,
    shape: i64,
    stride: i64,
}

and then maybe just

fn try_from(array: Arc<Mutex<Vec<T>>>) -> Result<DLPackTensor, Self::Error> {
    let guard = array.lock()?;           // single lock
    let shape = guard.len() as i64;      // extract while locked

    // SAFETY: guard borrows from Mutex inside Arc (heap-allocated).
    // Moving Arc doesn't invalidate the guard's internal pointer.
    // Field drop order: guard drops before _arc.
    let guard: MutexGuard<'static, Vec<T>> = unsafe {
        std::mem::transmute(guard)
    };

    let mut ctx = Box::new(ManagerContextMutex {
        guard, _arc: array, shape, stride: 1,
    });
    // ... extract pointers, build DLTensor
}

@Luthaf
Copy link
Copy Markdown
Member Author

Luthaf commented Mar 17, 2026

In my understanding, self referential structs are a very unsafe footgun, and I'd rather delegate support to an external crate for this.

Copy link
Copy Markdown
Member

@HaoZeke HaoZeke left a comment

Choose a reason for hiding this comment

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

Excellent.

@HaoZeke
Copy link
Copy Markdown
Member

HaoZeke commented Apr 9, 2026

Feel free to merge after the typo fix

Luthaf and others added 4 commits April 9, 2026 14:29
This allows sharing data that can be modified both by Rust and
the DlPack consumer, using locks to enforce borrowing rules at
runtime
The deleter functions were instantiated with the element type T (e.g.
f64) instead of the full Array<T, D> type. This caused Box::from_raw
in the deleter to reconstruct a Box with incorrect layout (size 32
instead of 88 for Array<f64, Ix2>), leading to undefined behavior on
deallocation.

The existing tests masked this bug by keeping an Arc::clone, so the
refcount never reached zero and the deleter never actually deallocated.
Add last-ref tests that pass ownership of the sole Arc into the tensor,
exercising the real deallocation path. Confirmed via miri.
There were a couple of violations of stacked borrow that
where fixed by using `Box<i64>` instead of inline i64 for the
shape/stride values.

Co-Authored-By: Guillaume Fraux <guillaume.fraux@epfl.ch>
@Luthaf Luthaf force-pushed the lock-everything branch from e7e563c to 8d1c9e0 Compare April 9, 2026 12:29
@Luthaf Luthaf merged commit d972cc0 into main Apr 9, 2026
6 checks passed
@Luthaf Luthaf deleted the lock-everything branch April 9, 2026 13:01
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