windy1 / zeroconf-rs

zeroconf is a cross-platform library that wraps underlying ZeroConf/mDNS implementations such as Bonjour or Avahi, providing an easy and idiomatic way to both register and browse services.
MIT License
77 stars 25 forks source link

Unsound AvahiMdnsService Drop implementation #4

Closed edgarogh closed 3 years ago

edgarogh commented 3 years ago

OS: Linux (Mint) Zeroconf version: 0.6.3

If an EventLoop is created from an AvahiMdnsService and the service is dropped, polling the event loop doesn't seem to do anything (it doesn't crash though) and the on_service_registered callback is never invoked.

MWE:

fn create_service(port: u16) -> (Option<MdnsService>, impl Fn() -> ()) {
    let mut service: AvahiMdnsService = MdnsService::new("_myapp._tcp", port);
    service.set_registered_callback(Box::new(|_, _| println!("Registered")));

    let event_loop = service.register().unwrap();

    let poll = move || event_loop.poll(Duration::from_secs(0)).unwrap();

    // If you replace the Option with None, "service" is dropped right there and:
    //   * The callback is not invoked
    //   * The service is not broadcasted on the network (but no crash/SIGSEGV here for some reason)
    // Currently, it works because the service is given back to the main function, and thus, lives until the loop breaks
    (Some(service), poll)
}

fn main() {
    let (_service, poll) = create_service(1337);

    loop {
        poll();
    }
}
edgarogh commented 3 years ago

In case some is in the same situation as me, the fix is to capture the service inside the closure's internal structure using a dummy expression statement (it must be a borrow if we want poll to be callable multiple times):

// fn create_service [...]

    let poll = move || {
        &service; // <-- THIS
        event_loop.poll(Duration::from_secs(0)).unwrap()
    };

    return poll;
}

This way, the service's drop() is bound to the closure's drop().

If you're not using a closure, return an explicit structure containing the service and the event loop.

windy1 commented 3 years ago

Thank you for bringing this to my attention, this is no longer possible with 0.7.0. The lifetime of EventLoop has been bound to the underlying poll struct for both Avahi and Bonjour so attempting to do something like this now will fail to compile. Here's a working example of your code:

fn main() {
    let mut service = create_service(1337);
    let event_loop = service.register().unwrap();

    let poll = move || event_loop.poll(Duration::from_secs(0)).unwrap();

    loop {
        poll();
    }
}

fn create_service(port: u16) -> MdnsService {
    let mut service = MdnsService::new("_myapp._tcp", port);
    service.set_registered_callback(Box::new(|_, _| println!("Registered")));
    service
}
edgarogh commented 3 years ago

Great, thanks for the quick reply & update ! Have a great day/evening/night.

windy1 commented 3 years ago

No problem, you too!