Add lifetimes to objects that contain callbacks

[depp]

This is probably a breaking change, so it needs a version bump.

Consider the following code:

use coremidi;

fn count_events(counter: &mut usize) {
    let source = coremidi::Source::from_index(0).unwrap();
    let client = coremidi::Client::new("Demo").unwrap();
    let callback = |packet_list: &coremidi::PacketList| {
        let n = packet_list.iter().count();
        println!("Event count: {}", n);
        *counter += n;
    };
    let input_port = client.input_port("Input", callback).unwrap();
    input_port.connect_source(&source).unwrap();
    let mut input_line = String::new();
    println!("Press Enter to Finish");
    std::io::stdin()
        .read_line(&mut input_line)
        .ok()
        .expect("Failed to read line");
}

fn main() {
    let mut counter = 0;
    count_events(&mut counter);
    println!("Total event count: {}", counter);
}

With the existing code, callback must be FnMut + 'static, but this is not really necessary. We just need callback to outlive the InputPort.

With the new code, callback can be FnMut + 'a and give you an InputPort<'a>. The same change is made to other types that contain callbacks.

[chris-zen]

@depp thank you very much for the contribution.

I'm not very proficient with lifetimes and I'd like other users to have a look into this too. @Boddlnagg, @jasongrlicky I'd love if you could have a look and help me asses the consequences of this change.

I thought that the static lifetime was needed for callbacks because they are used by real-time threads created by the CoreMIDI C library, which might call them for a very short time even when the port has been dropped. But I'm not really sure of that making any sense, and I would need to read the docs thoroughly again.

[Boddlnagg]

Pinging @kangalioo, because this sounds similar to Boddlnagg/midir#44

[kangalio]

The problem is that Rust allows leaking memory in safe code. Users can std::mem::forget(input_port), which will make the input port live forever - potentially outliving the scope it's meant for and causing UB and crashes etc.

[depp]

…the CoreMIDI C library, which might call them for a very short time even when the port has been dropped.

While not explicitly prohibited, this would make it impossible to free memory used by callbacks. A quick test adding sleep to the callback confirms this—when you destroy the port, Core MIDI will block until the callback finishes.

Users can std::mem::forget(input_port),…

I guess this change is no-go, if there is no guarantee that destructors are ever called. If we created the threads, we could at least choose to create them with something like crossbeam, but as it stands I have no idea how to get my app to work unless I start throwing unsafe around.

[depp]

Closing because I see no way to do this safely.

[chris-zen]

@depp if it helps, I have never needed to do something like what you show in your example, because MIDI callbacks happen in a real-time thread, which gives you very few margin to do things (no blocking calls at all, no allocation, no OS calls, no mutexes, ...). Then the only code that I write in the callback is for parsing the message and sending it through a ring buffer, for another thread to process it. Please see my code here. In that specific example, I pass the ownership of a handler to my MIDI driver, but the handler basically does what I told you, parsing and pushing to the ring buffer, then the data can be used somewhere else.

I hope it helps.

[chris-zen]

@Boddlnagg, @kangalioo thank you very much for your help.

[depp]

if it helps, I have never needed to do something like what you show in your example,...

The example code is just illustrative.

The problem is that it means that any callback has to communicate with the rest of the program through globals or unsafe pointer casts. But I understand now that this is a fundamental limitation of Rust's type system and just something you have to deal with.