`std::mem::forget` on a `MutexGuard`

[rickwebiii]

I'm building a DAG scheduler and have to guard against threads adding tasks while other threads are executing them. To this end, tasks look something like

struct Task {
  dependencies: AtomicUsize,
  dependents: Mutex<Vec<Arc<Task>>>,
  // blah blah
}

When creating a task, the task-generation thread adds itself to every non-finished dependency as follows:

for dep in deps {
    match dep.0.dependents.try_lock() {
        // If we acquire the lock, then add ourselves as a dependant and increase the
        // dependency count.
        Ok(mut x) => {
            new_task.dependencies.fetch_add(1, Ordering::Acquire);
            x.push(new_task.clone());
        }
        // If we fail to acquire the lock, then the dependent task has already completed
        // and notified its dependents. However, its data is immediately available for
        // use, so we don't update our dependency count.
        Err(TryLockError::WouldBlock) => {}
        _ => unreachable!(),
    }
}

// All dependencies are immediately available, schedule new_task on a threadpool
if new_task.dependencies.load(Ordering::Release) == 0 {
  //
}

When executing a task, another thread does the following:

// ... Execute the task

// At this point, our output has been written so we can notify our dependencies
// their data is available. Dependents contending on this lock can just back off
// and immediately use the data.
let mut deps = task.dependents.lock().unwrap();

// Notify our dependents that we've finished, which means our output buffer is
// available for use.
while let Some(dep) = deps.pop() {
    if dep.dependencies.fetch_sub(1, Ordering::Release) == 1 {
        // Dispatch `dep` on a threadpool
    }
}

// When this instruction retires, keep the mutex locked so any future dependents
// will just be able to immediately use our data.
std::mem::forget(deps);

The idea is that the task creation thread can check for dependency completion based on whether or not its dependents mutex is locked, and the std::mem::forget on the mutex guard call keeps it locked after the task finishes.

Is forgetting a MutexGuard a bad idea? In particular, does Rust guarantee anything about what happens when you drop the underlying Mutex while locked? pthread_mutex_destroy's documentation on Linux suggests this is undefined behavior, but Rust on Linux uses Futex, which is just an AtomicBool (and dropping it should be fine).

[quinedot]

@rickwebiii

Is forgetting a MutexGuard a bad idea?

Usually undesirable for what I presume are the obvious reasons around losing access to the data... interesting question as to the further implications though.

@rickwebiii

In particular, does Rust guarantee anything about what happens when you drop the underlying Mutex while locked?

I didn't find any guarantees about this in the documentation. I doubt there are any.

@rickwebiii

pthread_mutex_destroy's documentation on Linux suggests this is undefined behavior, but Rust on Linux uses Futex, which is just an AtomicBool (and dropping it should be fine).

Those are implementation details. However, you can forget a MutexGuard with safe code, so UB cannot result... unless there's a bug in std.

...cough...

Turns out, this used to be a bug in std. Now it leaks the pthread_* types instead, on platforms that still use them.^[1]

But that's still an implementation detail. So your approach might leak memory on some systems today, or on other systems tomorrow, or might do something else to avoid UB if necessary and possible, etc.

---

1. Unless something has changed since that PR; that's where I stopped looking. ↩︎

[kornel]

If I understand correctly, you're able to split this work into two phases, so that you first finish writing, and then start or unpause threads that will read it?

Could you use once_cell instead? It will let you initialize data once, even if other threads can reference it.

And if you can avoid giving other threads a reference ahead of time (e.g. don't spawn them yet, or send data via channel) then you should be able to avoid Mutex entirely and initialize data via &mut.

[rickwebiii]

Futex is basically the ideal type for this pattern, but it's unfortunate that I can't guarantee its existence. I can probably use a CAS spinlock here and live with the perf hit that come with not informing the OS what you're waiting for. Lock contention should be effectively zero here.

[khimru]

@rickwebiii

Futex is basically the ideal type for this pattern, but it's unfortunate that I can't guarantee its existence.

If you want to use futex then why don't just use futex? Rust couldn't guarantee that futex is always available because many platforms don't provide anything of the sort! In fact Windows7 without support for futex analogue was still tier1 till very recently and MacOS, that doesn't have an equivalent, is still tier1.

If you only exclusively need or want something that exists on one or two platforms usually the best way is to just access low-level facilities directly, rather then rely on implementation details which may change with time.

[user16251]

Mutex<Vec<T>> and Mutex<VecDeque<T>> are code smells for a channel but unfortunately crossbeam is missing the one operation this needs. tokio channels have a close operation that doesn't allow further sending but facilitates draining the receiver which corresponds to the lock-iterate-forget here. But AFAICT you can't do that in crossbeam; as long as you're receiving, other threads can keep sending.

[user16251]

Although I should add that the OP wakes up tasks in LIFO order but a channel-based solution would be FIFO.