alexcrichton / curl-rust

Rust bindings to libcurl
MIT License
1k stars 234 forks source link

Fix Multi::messages() signature #499

Closed nabijaczleweli closed 1 year ago

nabijaczleweli commented 1 year ago

The current

pub fn messages<F>(&self, mut f: F)
where
    F: FnMut(Message),
{
    self._messages(&mut f)
}

fn _messages(&self, f: &mut dyn FnMut(Message)) {
    let mut queue = 0;
    unsafe {
        loop {
            let ptr = curl_sys::curl_multi_info_read(self.raw.handle, &mut queue);
            if ptr.is_null() {
                break;
            }
            f(Message { ptr, _multi: self })
        }
    }
}

one meant that

let mut errors = vec![];
sucker.messages(|m| errors.push(m));

was illegal because

error[E0521]: borrowed data escapes outside of closure
    --> src\ops\mod.rs:1163:33
     |
1162 |             let mut errors = vec![];
     |                 ---------- `errors` declared here, outside of the closure body
1163 |             sucker.messages(|m| errors.push(m));
     |                              -  ^^^^^^^^^^^^^^ `m` escapes the closure body here
     |                              |
     |                              `m` is a reference that is only valid in the closure body

If it's not obvious at first glance, this is because Message is actually Message<'multi>, and the inferred lifetime was, expectedly, matching the &mut self of the FnMut (or close enough).

Instead, spec messages<'s>(&'s self) with FnMut(Message<'s>), which fixes the above snippet by binding the 'multi to the Multi.

alexcrichton commented 1 year ago

This is currently intentional that the messages are only valid for the closure iteration. The documentation states that the returned pointer doesn't survive curl_multi_remove_handle which can be called on &self. You'll need to copy data out of the Message to push it onto your vector.

nabijaczleweli commented 1 year ago

yep, actually using this revealed that it segfaults (but not when being traced, natch)