Help me XY: How can I call pthread_kill without TOCTOU risk

help
1

I really hope this can be XY-ed into something else (as that will likely also provide options for better UX in the future).

I need to kill some background threads that I spawned. I don't want to risk TOCTOU between JoinHandle::is_finished() and libc::pthread_kill / TerminateThread. I'd also really like to be able to do this in a vaguely sane manner (not so abrupt as to risk other UB from partial disk writes etc. but I don't need to worry about any logical invariants).

(I can't find the rust = good, sane, safe approach to this usual GUI concern)

I have a GUI running on my main thread and a couple of workers running in the background. The basic form is

fn main() -> Result<_,_> {
    let app = Ripper::new()?;
    // ... some Setup work ...

    // clicking "Rip" sends a Vec of selected tracks down this channel
    let (to_rip_tx, to_rip_rx) = mpsc::channel::<Vec<usize>>();
    let (ripped_tx, ripped_rx) = mpsc::channel::<RippedTrack>();

    let ripper = thread::spawn(move || {
        while let Ok(tracks) = to_rip_rx.recv() {
            for track_number in tracks {    
                // ... read raw data from CD ...
                ripped_tx.send(data)?;
            }
        }
    });

    let encoder = thread::spawn(move || {
        while let Ok(track) = ripped_rx.recv() {
            // ... encode, tag, save ...
        }
    });

    app.run()?;
    drop(app);
    ripper.join()??;
    encoder.join()?;
}

My issue is that when the user "closes" the app (by closing the GUI) the ripper & encoder threads stay active until all the selected tracks are processed. Only then does the program complete. Semantically closing the GUI should stop all active ripping "immediately"[1]

Having a "nice" way to do this would also allow for a "Cancel" button. It's not async and really works best as threads, so moving to futures which "I could just drop" would bring much more pain than gain (says my gut)

  1. for a sane definition of "immediately" - so finish writing a file that is in the process of being written to disk, but stop reading the CD after the current chunk and not start writing any files to disk which are not already in the process of being written, etc... ↩︎

2

TOCTOU with the thread completing is insignificant next to the bigger problem that it is not sound to externally terminate a thread — unless you wrote literally all the code that thread executes and know that it will never lock a shared mutex, pass a borrowed value to another thread, or do anything else that is problematic under termination.

The only practical sound option is to put code in the thread that checks a flag telling it to stop its work. Yes, this is disruptive, but those check-points are what is necessary to make stopping sound.

3

You could use an enum for the channels, with one variant carrying the data, and another one carrying the "stop" signal. No need for killing anything if the app sends the stop variant on the channels.

4

That’s not a thing. Once the kernel has control, a given write either fully works or fully fails barring a hardware failure. Killing the calling thread is irrelevant.

A two-phase commit avoids partial file problems.

5

On Unix write syscalls can't be interrupted (uninterruptable sleep) Linux as an exception does allow SIGKILL for the entire process during uninterruptable sleep nowadays.

6 
#![feature(mpsc_is_disconnected)]    // <- requires nightly

while let Ok(tracks) = to_rip_rx.recv() {
    for track_number in tracks {
        // We stop the iteration over `tracks`, the outer loop will
        // continue to consume (and discard) all buffered messages.
        if to_rip_rx.is_disconnected() { break; }

        // ... read raw data from CD, send forth ...
    }
}
7

Which is why I'm hoping for suggestions on how to do that idiomatically. The approach should at least run drop properly through the stack; so panic is OK, anything equivalent to process::exit is not.

The majority of // ... read raw data from CD ... is a single, long-running call to a library function. At least I own that library too so can make adjustments if I can find a reasonable API.

So far the best options I can come up with since posting this are:

  1. use a pre-existing crate to handle this and adjust lib and bin accordingly
  2. create my own crate and expose something along the lines of:
    // setup early in main()
let (workerthreads: Controller, keepalive: Context) = Context::new();

    // from the controlling (GUI) thread
workerthreads.cancel(); // drops an `mpmc::Sender` - preferred
workerthreads.panic(); // sets an `AtomicBool` - worth having as an option

    // in suitable places within the lib
for chunk in big_bit_of_work { try {
     keepalive?;
}}
  3. just don't join the threads at the end, then when the GUI exits, the process terminates and the OS will handle closing them. But other than "don't leave threads unjoined" I can't find any documentation detailing whether this will just hard-abort or unwind them, I'm guessing hard-abort.
8

That wasn’t clear from your post’s introduction or its title “How can I call pthread_kill without TOCTOU risk”. If the problem with killing threads was just the TOCTOU you mentioned, then all you’d need to do is use a mutex that ensures you don’t attempt to kill the thread when it’s planning to exit — but it very much isn’t.

As to cleanly stopping a thread, there really isn’t a particular idiom or pattern for this in Rust, as far as I know. The ways to do it include:

  • If the thread is reading data from a channel, close the channel. (In your case, the channel’s items are coarser than you want, so this doesn’t help.)
  • Write async code (and be sure to yield regularly if the IO the thread does doesn’t suffice).
  • If nothing else is available, check a AtomicBool regularly.
    • If you are calling code you don’t control, you still may be able to inject such checks if there is any kind of callback; for example, if it Reads some data then provide a custom Read implementation that checks whether to stop.

The way this will fit well into your thread’s code depends entirely on what that thread is doing. The reason there is no fully general solution is the same reason that pthread_kill can’t be used without special care: arbitrary code doesn’t have sound cancellation points, so you have to introduce some.

9

It's not async and really works best as threads, so moving to futures which "I could just drop" would bring much more pain than gain (says my gut)

Another option is to keep the different threads (as you're presumably also doing compute workloads to convert the data in addition to doing a lot of IO to read it), but using a single-thread async runtime (e.g. smol) on each thread to allow cancelling.

most of the existing code could likely be kept mostly unchanged except for replacing blocking functions with their async counterparts and yielding between expensive computations and inside long loops (to allow breaking earlier).

for checking for cancellation in parallel to running the loop you could just use race, e.g.:

race(
    async {
        wait_for_cancellation_from_main().await;
        
        Ok(())
    },
    async {
        // make sure there's always at least 1 cancellation point
        // per loop so that the thread can't get stuck in a loop.
        while let Ok(tracks) = to_rip_rx.recv().await {
            for track_number in tracks {    
                // ... read raw data from CD ...
                ripped_tx.send(data)?.await;
            }
        }
    }
)

you could also use loop select, but that has weird cancellation safety issues and is overkill for this use case unless your code grows more complex and needs to respond to multiple streams from the samme thread.
if you do end up using loop select there's also floop (made by me), which is essentially loop select without the cancellation safety issues.