microsoft / windows-rs

Rust for Windows
https://kennykerr.ca/rust-getting-started/
Apache License 2.0
10.13k stars 473 forks source link

No good way to use `IDebugEventCallbacks` with the rust type system #3061

Closed Ben-Lichtman closed 1 month ago

Ben-Lichtman commented 1 month ago

Summary

While writing a custom debugger, I have something that looks like

struct MyDbg { client: IDebugClient5, state: SomeStateStuff }

I need to set callbacks using IDebugClient5::SetEventCallbacks which requires creating some IDebugEventCallbacks, the problem is that these callbacks need to be able to modify state somehow so that I can extract event information back out of the MyDbg. In c++ the solution would be to implement IDebugEventCallbacks on the MyDbg and pass that into the client, however it seems like this is not possible in rust, as the only way to construct a IDebugEventCallbacks is to call .into(), which consumes the concrete struct.

so I have the problem looking a bit like this

struct MyDbg { client: IDebugClient5, state: SomeStateStuff }

let dbg : MyDbg = ...;
dbg.client.SetEventCallbacks(dbg.into());
dbg.client.Whatever(...); // error: dbg was consumed by `dbg.into()

The current solution to this is to use an inner Arc like

#[derive(Clone)]
struct MyDbg(Arc<MyDbgState>);

so I can freely clone it then .into() on the clone, however it doesn't seem like there's any way to do this without using an Arc - for example by getting a &MyDbg or *const MyDbg out from an IDebugEventCallbacks.

Crate manifest

No response

Crate code

No response

sivadeilra commented 1 month ago

I think the recent work I've been doing on ComObject<T> may help you solve exactly this problem. Basically, there is a new step, which is "convert my T into a ComObject", which you invoke by calling ComObject::new(my_app). This moves your value into the heap (into a reference-counted container) and then returns a ComObject<T>, which is an owned reference to your object, in the heap.

ComObject<T> provides methods for getting access to the interface chains. See the ComObject::to_interface method, mainly. ComObject has a Clone impl that effectively just calls AddRef for you.

You will still need to deal with interior mutability. Since COM objects inherently allow more than one reference, the methods receive &self, never &mut self. So you'll need to use something like RefCell or Mutex to do runtime checks for exclusive ownership of the contents of a COM object.

Ben-Lichtman commented 1 month ago

@sivadeilra this sounds ideal! I'm not sure where to find ComObject though. Plz could you point me to the docs / module, thanks (assuming this feature is currently live?)

sivadeilra commented 1 month ago

It's only available in the Git repo, for now. I've been making a bunch of improvements for COM support, and we haven't published them yet because I didn't want to repeatedly publish crates, when I know that I'm just going to submit another PR soon.

My work is about to slow down, because I've accomplished most of the improvements I wanted. As soon as I land a few more features / improvements for COM support, we will publish a new version of all of the Windows crates.

Until then, you can directly use the Git repo. Something like:

[dependencies.windows]
git = "https://github.com/microsoft/windows-rs"
rev = "8b4a185e56c1355ad0f4908d9e5e9f338b5c1360"

Or you can use a Cargo config.toml file to override your dependencies and point at a local Git clone of this repo.

I don't have an exact schedule of when these changes will be published, but I'd say within the next 2 weeks. Meanwhile, for docs, you can skim the sources at /crates/libs/core/src/com_object.rs.

Ben-Lichtman commented 1 month ago

Seems to me that this should still be classified as an open bug until the relevant APIs are added into the crate?

kennykerr commented 1 month ago

The policy for windows-rs is to close issues once there is no more left to be done. Anyway, there was a release a few days ago:

https://github.com/microsoft/windows-rs/releases/tag/0.57.0

Does this have what you're looking for?

Ben-Lichtman commented 1 month ago

Ah you're right, this version does have the new API, awesome!