Fix segfault when dropping multi handle with active transfers

[sagebind]

Fix a potential segfault that can occur when dropping a Multi before dropping an Easy2Handle that is attached to the multi handle and has an active transfer. This can happen because dropping a Multi will also always drop its inner MultiData, which holds socket and timer callback functions supplied by the user, but the underlying CURLM handle is not dropped until both the Multi and all of its attached Easy2Handles are dropped. If the Multi is dropped first, then the socket and timer callback functions will be dropped while the CURLM handle is still active, which can lead to a segfault if the callbacks are invoked after they have been dropped.

The libcurl docs have this to say about CURLMOPT_SOCKETFUNCTION:

This callback may get invoked at any time when interacting with libcurl. This may even happen after all transfers are done and is likely to happen during a call to curl_multi_cleanup when cached connections are shut down.

This paragraph is clear that libcurl may invoke our CURLMOPT_SOCKETFUNCTION callback when curl_multi_cleanup is invoked, which in our case, is done by RawMulti::drop which might be called after we've already freed our callback function.

To fix this, use Rc and RefCell to extend the life of the configured callbacks until both the Multi and all attached Easy2Handles are dropped, avoiding the need to unset the callbacks until all socket activity is done. (MultiWaker still theoretically may cause the callbacks to be invoked, so we still unset them.)

Generally I would avoid RefCell as a way of solving lifetime problems, but in this case it seems like RefCell models libcurl's actual mutability contract decently well, so I think it works.

---

This issue can be easily reproduced with the following program, which ends in a SIGSEGV 100% of the time on my machine:

use curl::{easy::Easy, multi::Multi};
use std::time::Duration;

fn main() {
    // Set up basically any request.
    let mut easy = Easy::new();
    easy.url("https://example.org").unwrap();

    // Set up a multi with any socket callback.
    let mut multi = Multi::new();
    multi.socket_function(|_, _, _| {}).unwrap();

    // Attach the easy handle to the multi handle.
    let handle = multi.add(easy).unwrap();

    // Execute the initial steps of establishing a socket, but without waiting
    // for the request to fully finish.
    multi.perform().unwrap();
    multi.wait(&mut [], Duration::from_secs(1)).unwrap();
    multi.perform().unwrap();

    // Drop the multi handle. This will free the callback functions, leaving
    // curl with dangling pointers.
    drop(multi);

    // Segfault when curl tries to call the socket callback.
    drop(handle);
}

Thanks to @joelposti for reporting this issue on sagebind/isahc#459 and supplying a larger reproduction of the issue with detailed steps, which made it much easier for me to find the root cause here.

[sagebind]

I've updated this PR with an alternative implementation. This implementation is a bit less minimal and uses Rc and RefCell to extend the life of the configured callbacks until both the Multi and all attached Easy2Handles are dropped, avoiding the need to unset the callbacks until all socket activity is done. (MultiWaker still theoretically may cause the callbacks to be invoked, so we still unset them.)

Generally I would avoid RefCell as a way of solving lifetime problems, but in this case it seems like RefCell models libcurl's actual mutability contract decently well, so I think it works. However, there are two scenarios where this causes a problem:

• Attempting to change your callbacks from within your callback. Our lifetimes don't allow this anyway since your callback would need a mutable reference to your Multi, which itself is required to set the callback in the first place. Though, one could call unsafe libcurl functions directly and attempt this, resulting in an error from an already-borrowed RefCell.
• Recursive calls into Multi attempting to call your callback while your callback is already running higher in the stack. This would cause a RefCell error, but again, lifetimes disallow this if using the safe API.

Note that Multi is already !Send so using Rc here to hold onto the callbacks fits well.

You can compare the new implementation with the old one:

• https://github.com/alexcrichton/curl-rust/pull/645/changes
• bdc9016