impl _wmi module#7162
Merged
Merged
Conversation
Implement _wmi.exec_query via COM/WMI using pipe+thread pattern matching _wmimodule.cpp. Uses raw COM vtable calls for IWbemLocator, IWbemServices, IEnumWbemClassObject, and IWbemClassObject interfaces.
Contributor
📝 WalkthroughWalkthroughA new Windows-specific WMI query module is introduced with PyO3 FFI bindings for COM/Win32 APIs. The module exports a single Changes
Sequence DiagramsequenceDiagram
participant Python as Python VM
participant Rust as Rust Module
participant Thread as WMI Thread
participant COM as Windows COM
participant Pipe as Named Pipe
Python->>Rust: exec_query(query_string)
Rust->>Rust: Validate query (SELECT check)
Rust->>Thread: Spawn query execution thread
Rust->>Pipe: Create pipe for results
Thread->>COM: CoInitializeEx()
Thread->>COM: CoCreateInstance (WbemLocator)
Thread->>COM: ConnectServer (ROOT\CIMV2)
Thread->>COM: ExecQuery (WQL)
Thread->>COM: Enumerate class objects
loop For each WMI object
Thread->>COM: GetNames (properties)
Thread->>COM: Get (property values)
Thread->>Pipe: Write name=value\0
end
Thread->>COM: Release objects & cleanup
Rust->>Pipe: Read all results
Rust->>Python: Return concatenated string
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
📦 Library DependenciesThe following Lib/ modules were modified. Here are their dependencies: [x] test: cpython/Lib/test/test_wmi.py dependencies: dependent tests: (no tests depend on wmi) Legend:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@crates/vm/src/stdlib/_wmi.rs`:
- Around line 575-582: The HANDLE variables h_thread, read_pipe, and write_pipe
are initialized with null_mut() and later checked with .is_null(), but
windows-sys 0.61.2 defines HANDLE as an integer (isize), so replace
pointer-specific usage: initialize these HANDLEs to 0 instead of null_mut() and
change any .is_null() checks (e.g., on read_pipe, write_pipe, h_thread) to
comparisons against 0 (== 0); alternatively, you can use
MaybeUninit::<HANDLE>::uninit() for these symbols to avoid direct zeroing—update
code around the HANDLE declarations and all occurrences of .is_null() for HANDLE
values accordingly (refer to h_thread, read_pipe, write_pipe, and any other
HANDLE variables in this file).
- Around line 466-532: The loop leaks prop_name (BSTR) and prop_value (VARIANT)
for system-flavor properties because VariantClear and SysFreeString are only
called inside the flavor-handling block; after object_next returns you must
always free the allocated resources even when (flavor & WBEM_FLAVOR_MASK_ORIGIN)
== WBEM_FLAVOR_ORIGIN_SYSTEM or on non-handled branches. Fix: after each
object_next call (and after using prop_value/prop_name when succeeded), move the
cleanup calls (VariantClear(&mut prop_value) and SysFreeString(prop_name)) out
of the flavor-specific if so they run unconditionally (guarding
SysFreeString(prop_name) for null), and ensure cleanup still happens on early
breaks/errors from object_next; reference variables and functions: object_next,
prop_name, prop_value, flavor, WBEM_FLAVOR_ORIGIN_SYSTEM, VariantClear,
SysFreeString.
🧹 Nitpick comments (1)
crates/vm/src/stdlib/_wmi.rs (1)
671-673: Thread HRESULT exit codes may produce misleadingOSErrormessages.When the thread returns a failure HRESULT (e.g.
0x80040154for "class not registered"), it's stored asu32and eventually passed tostd::io::Error::from_raw_os_error(err as i32)at line 688.from_raw_os_errorinterprets this as a Win32 error code, not an HRESULT, producing an incorrect error message. This matches CPython's behavior, so it's likely intentional, but worth a brief note for future maintainers.
Comment on lines
+466
to
+532
| while succeeded(hr) { | ||
| let mut prop_name: *mut u16 = null_mut(); | ||
| let mut prop_value = VARIANT::zeroed(); | ||
| let mut flavor: i32 = 0; | ||
|
|
||
| hr = object_next( | ||
| value, | ||
| 0, | ||
| &mut prop_name, | ||
| &mut prop_value, | ||
| null_mut(), | ||
| &mut flavor, | ||
| ); | ||
|
|
||
| if hr == WBEM_S_NO_MORE_DATA { | ||
| hr = 0; | ||
| break; | ||
| } | ||
|
|
||
| if succeeded(hr) | ||
| && (flavor & WBEM_FLAVOR_MASK_ORIGIN) != WBEM_FLAVOR_ORIGIN_SYSTEM | ||
| { | ||
| let mut prop_str = [0u16; BUFFER_SIZE]; | ||
| hr = | ||
| VariantToString(&prop_value, prop_str.as_mut_ptr(), BUFFER_SIZE as u32); | ||
|
|
||
| if succeeded(hr) { | ||
| let cb_str1 = (wcslen(prop_name) * 2) as u32; | ||
| let cb_str2 = (wcslen(prop_str.as_ptr()) * 2) as u32; | ||
|
|
||
| if WriteFile( | ||
| write_pipe, | ||
| prop_name as *const _, | ||
| cb_str1, | ||
| &mut written, | ||
| null_mut(), | ||
| ) == 0 | ||
| || WriteFile( | ||
| write_pipe, | ||
| &eq_sign as *const u16 as *const _, | ||
| 2, | ||
| &mut written, | ||
| null_mut(), | ||
| ) == 0 | ||
| || WriteFile( | ||
| write_pipe, | ||
| prop_str.as_ptr() as *const _, | ||
| cb_str2, | ||
| &mut written, | ||
| null_mut(), | ||
| ) == 0 | ||
| || WriteFile( | ||
| write_pipe, | ||
| &null_sep as *const u16 as *const _, | ||
| 2, | ||
| &mut written, | ||
| null_mut(), | ||
| ) == 0 | ||
| { | ||
| hr = hresult_from_win32(GetLastError()); | ||
| } | ||
| } | ||
|
|
||
| VariantClear(&mut prop_value); | ||
| SysFreeString(prop_name); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Resource leak: prop_name (BSTR) and prop_value (VARIANT) are not freed for system-flavor properties.
object_next allocates prop_name and fills prop_value for every property, but VariantClear/SysFreeString at lines 529-530 are only called inside the if succeeded(hr) && !system_flavor block. When a property has WBEM_FLAVOR_ORIGIN_SYSTEM, the entire block is skipped and both resources leak — once per system property, per WMI object.
🐛 Proposed fix — move cleanup outside the flavor check
- if succeeded(hr)
- && (flavor & WBEM_FLAVOR_MASK_ORIGIN) != WBEM_FLAVOR_ORIGIN_SYSTEM
- {
+ if succeeded(hr) {
+ if (flavor & WBEM_FLAVOR_MASK_ORIGIN) != WBEM_FLAVOR_ORIGIN_SYSTEM {
let mut prop_str = [0u16; BUFFER_SIZE];
hr =
VariantToString(&prop_value, prop_str.as_mut_ptr(), BUFFER_SIZE as u32);
if succeeded(hr) {
let cb_str1 = (wcslen(prop_name) * 2) as u32;
let cb_str2 = (wcslen(prop_str.as_ptr()) * 2) as u32;
if WriteFile(
write_pipe,
prop_name as *const _,
cb_str1,
&mut written,
null_mut(),
) == 0
|| WriteFile(
write_pipe,
&eq_sign as *const u16 as *const _,
2,
&mut written,
null_mut(),
) == 0
|| WriteFile(
write_pipe,
prop_str.as_ptr() as *const _,
cb_str2,
&mut written,
null_mut(),
) == 0
|| WriteFile(
write_pipe,
&null_sep as *const u16 as *const _,
2,
&mut written,
null_mut(),
) == 0
{
hr = hresult_from_win32(GetLastError());
}
}
+ }
VariantClear(&mut prop_value);
SysFreeString(prop_name);
}🤖 Prompt for AI Agents
In `@crates/vm/src/stdlib/_wmi.rs` around lines 466 - 532, The loop leaks
prop_name (BSTR) and prop_value (VARIANT) for system-flavor properties because
VariantClear and SysFreeString are only called inside the flavor-handling block;
after object_next returns you must always free the allocated resources even when
(flavor & WBEM_FLAVOR_MASK_ORIGIN) == WBEM_FLAVOR_ORIGIN_SYSTEM or on
non-handled branches. Fix: after each object_next call (and after using
prop_value/prop_name when succeeded), move the cleanup calls (VariantClear(&mut
prop_value) and SysFreeString(prop_name)) out of the flavor-specific if so they
run unconditionally (guarding SysFreeString(prop_name) for null), and ensure
cleanup still happens on early breaks/errors from object_next; reference
variables and functions: object_next, prop_name, prop_value, flavor,
WBEM_FLAVOR_ORIGIN_SYSTEM, VariantClear, SysFreeString.
Contributor
There was a problem hiding this comment.
@youknowone, understood. I'll defer to your judgment on this design decision.
youknowone
added a commit
to youknowone/RustPython
that referenced
this pull request
Mar 22, 2026
impl _wmi module
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit