Bug: IMapView.Lookup fails with 0x0 when key DOES exist

[jaysonsantos]

Which crate is this about?

windows

Crate version

0.48.0

Summary

Hi there, firstly, thanks for the effort on creating these bindings, they are really helpful to write safe programs!

I noticed that IMapView Lookup's method, will always fail when it does contain a key, but that only happens on IMapView returned from DeviceWatcher and it does not happen if you use TryFrom to convert a BTreeMap into IMapView and then Lookup the key.

Toolchain version/configuration

Default host x86_64-pc-windows-msvc
rustup home: USER.rustup

stable-x86_64-pc-windows-msvc (default)
rustc 1.70.0 (90c541806 2023-05-31)

Reproducible example

use std::{
    sync::{
        atomic::{AtomicBool, Ordering},
        Arc,
    },
    thread::sleep,
    time::Duration, collections::BTreeMap,
};

use windows::{
    core::{HSTRING, Result},
    h,
    Devices::Enumeration::{
        DeviceClass, DeviceInformation, DeviceInformationUpdate, DeviceWatcher,
    },
    Foundation::{
        Collections::{IIterable, IMapView},
        TypedEventHandler,
    },
};

fn main() -> Result<()> {
    let started = Arc::new(AtomicBool::new(false));
    let cloned = started.clone();

    let parent_property = h!("System.Devices.Parent");
    let additional_properties: IIterable<HSTRING> = vec![parent_property.clone()].try_into()?;
    let query = DeviceInformation::GetAqsFilterFromDeviceClass(DeviceClass::VideoCapture)?;
    let watcher = DeviceInformation::CreateWatcherAqsFilterAndAdditionalProperties(
        &query,
        &additional_properties,
    )?;
    watcher.Added(&TypedEventHandler::<DeviceWatcher, DeviceInformation>::new(
        move |_, info| {
            if let Some(device) = info {
                let parent_property = h!("System.Devices.Parent");

                let properties = device.Properties()?;
                dbg!(properties.HasKey(parent_property)?);
                dbg!(properties.Lookup(parent_property));
            }
            Ok(())
        },
    ))?;
    watcher.Removed(
        &TypedEventHandler::<DeviceWatcher, DeviceInformationUpdate>::new(move |_, info| {
            if let Some(info) = info {
                let id = info.Id()?.to_string_lossy();
                eprintln!("Removed {}", id);
            }
            Ok(())
        }),
    )?;

    watcher.EnumerationCompleted(&TypedEventHandler::<DeviceWatcher, _>::new(move |_, _| {
        cloned.store(true, Ordering::SeqCst);
        Ok(())
    }))?;
    watcher.Start()?;

    let mut tries = 0;

    while !started.load(Ordering::Relaxed) {
        sleep(Duration::from_secs(1));
        tries += 1;

        if tries >= 5 {
            panic!("Timeout");
        }
    }

    let mut map = BTreeMap::new();
    map.insert(parent_property.clone(), h!("test").clone());
    let map: IMapView<HSTRING, HSTRING> = map.try_into()?;

    assert_eq!(dbg!(map.Lookup(parent_property)), Ok(h!("test").clone()));
    Ok(())
}

Crate manifest

[package]
name = "x"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
windows = { version = "0.48.0", features = ["Devices_Enumeration", "Foundation", "Foundation_Collections", "implement"] }

Expected behavior

Lookup should return Ok(I-something)

Actual behavior

This gets printed:

[src/main.rs:40] properties.HasKey(parent_property)? = true
[src/main.rs:41] properties.Lookup(parent_property) = Err(
    Error {
        code: HRESULT(0x00000000),
        message: "The operation completed successfully.",
    },
)
[src/main.rs:40] properties.HasKey(parent_property)? = true
[src/main.rs:41] properties.Lookup(parent_property) = Err(
    Error {
        code: HRESULT(0x00000000),
        message: "The operation completed successfully.",
    },
)
[src/main.rs:77] map.Lookup(parent_property) = Ok(
    "test",
)

Additional comments

No response

[kennykerr]

I'm not familiar with this API but looking at Properties it returns an IMapView<string, object> so if the "value" in the map is "null" then methods like Lookup will return an Err with a success code since there is no value to return.

[kennykerr]

Here's an example of how this works:

[dependencies.windows]
version = "0.51"
features = [
    "implement",
    "Foundation_Collections",
]

use std::collections::*;
use windows::{core::*, Foundation::Collections::*, Foundation::*};

fn main() -> Result<()> {
    let object: IInspectable = 123.try_into()?;

    let mut map = BTreeMap::new();
    map.insert(h!("some").clone(), Some(object));
    map.insert(h!("none").clone(), None);

    let map: IMapView<HSTRING, IInspectable> = map.try_into()?;

    let some = map.Lookup(h!("some")).expect("should be Ok");
    assert_eq!(123, some.cast::<IReference<i32>>()?.Value()?);

    let none = map.Lookup(h!("none")).expect_err("should be Err");
    assert_eq!(Error::OK, none);
    assert_eq!(HRESULT(0), none.code());

    Ok(())
}

[kennykerr]

Beyond that, I'd recommend the following resources for API questions:

• https://docs.microsoft.com/en-us/answers/topics/windows-api.html
• https://stackoverflow.com/

Also, once I have completed #332 it should be a lot simpler to wire up the event handlers. 😊

[jaysonsantos]

@kennykerr thanks for the clarification.
Would you say it is possible that methods like Lookup, Get, Pop could return Result<Optional<T>>? for semantics or that is something tied to the automated codegen?

[kennykerr]

Yes, the trouble is knowing which are likely to be nullable vs not. At this point, all are theoretically nullable but then you end up having to unwrap both the Result and the Option and I found that becomes very tedious in practice since most APIs will return a non-null value.