Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delegates now generated with return bound to static lifetime, is that right? #1110

Closed
riverar opened this issue Sep 3, 2021 · 5 comments
Closed
Labels
enhancement New feature or request

Comments

@riverar
Copy link
Collaborator

riverar commented Sep 3, 2021

My Rust lifetime foo is weak so be gentle.

A recent change was made to delegate generation; delegates now appear to be generated with a return that is bound to 'static lifetime.

quote! { F: FnMut(#(#params),*) -> ::windows::Result<#return_sig> + 'static }

Is this right? If so, why?

This change appears to make it almost impossible to create a closure for things like RoutedEventHandler that reference, say, self with an implicit anonymous lifetime, e.g. consider:

fn OnLaunched(&mut self, _: &Option<LaunchActivatedEventArgs>) -> windows::Result<()> {
...
  button.Click(RoutedEventHandler::new(|sender, args| {
      self.ButtonClicked(...)
  }))?;
}

0.19 will err with error[E0759]: `self` has an anonymous lifetime `'_` but it needs to satisfy a `'static` lifetime requirement. Removing the explicit lifetime in delegate.rs (the line above) unblocks me, but I haven't fully thought through the implications here and would love another set of brains on it.

@riverar riverar added the question Further information is requested label Sep 3, 2021
@kennykerr
Copy link
Collaborator

Was it not static before? I was consolidating a lot of similar code and may have overly restricted this case in the process.

But the lifetime here is a bit tricky. The handler must capture either strong or weak references and self is neither. The delegate may outlive (in theory) the object surrounding self. In C++/WinRT I introduced the make_weak and make_strong helpers to turn this into something that can be carried along with a handler. We probably need something similar for Rust.

@Paul-Dempsey
Copy link

This issue is painfully limiting in the app I'm trying to write. I'm not a rust guru, so looking for a viable strategy to be able to &mut self in event handlers.

@riverar
Copy link
Collaborator Author

riverar commented Nov 24, 2022

@Paul-Dempsey Here's a pattern I've used since my original issue:

struct App {
  _object: RefCell<Option<ObjectTypeFoo>>
  // ...
}

impl App {
  fn new() -> Self { _object: RefCell::new(None) }
  // ...
}

impl IFoo for App {
  fn OnLaunched(&self, ...) {
    *self._object.borrow_mut() // <-- Do something with mutable borrow here
  }
}

@Paul-Dempsey
Copy link

Paul-Dempsey commented Nov 24, 2022

Thank you, @riverar . A concrete example with TypedEventHandler that compiles would be more helpful (this code doesn't compile, and a bit abstract for a newbie). What's an ObjectTypeFoo? I'd love to see a version of the rust\windows-rs\crates\samples\device_watcher\ that shows how to reference self from the handler (e.g. dynamically maintain a list of devices by name as they are added and removed), rather than trivial context-free handlers that print something.

@riverar
Copy link
Collaborator Author

riverar commented Jan 24, 2023

Closing as the patterns really do help you write better multithreaded COM code (#2304 (comment)).

@Paul-Dempsey Here's the sample you requested. I suspect there are several other Rust idiomatic approaches you can probably take here.

use std::sync::{Arc, Mutex};
use windows::{Devices::Enumeration::DeviceInformation, Foundation::TypedEventHandler};

fn main() -> windows::core::Result<()> {
    let state = Arc::new(Mutex::from(0));

    let clone = state.clone();
    let watcher = DeviceInformation::CreateWatcher()?;
    watcher.Added(&TypedEventHandler::new(move |_, _info| {
        let mut state = clone.lock().unwrap();
        *state += 1;
        Ok(())
    }))?;
    watcher.Start()?;

    std::thread::sleep(std::time::Duration::new(3, 0));
    println!("Devices added: {}", state.lock().unwrap());

    Ok(())
}

@riverar riverar closed this as not planned Won't fix, can't repro, duplicate, stale Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants