Skip to content

Remove Drop impl on VARIANT and PROPVARIANT#3886

Merged
kennykerr merged 2 commits intomasterfrom
rafael/drop-variant-drop
Feb 11, 2026
Merged

Remove Drop impl on VARIANT and PROPVARIANT#3886
kennykerr merged 2 commits intomasterfrom
rafael/drop-variant-drop

Conversation

@riverar
Copy link
Copy Markdown
Collaborator

@riverar riverar commented Feb 10, 2026

VARIANT and PROPVARIANT previously implemented Drop by unconditionally calling VariantClear/PropVariantClear. This created a problem as ownership of these types is context-dependent. In particular, some implementations of COM interfaces (e.g., IUIAutomationPropertyChangedEventHandler) receive [in] VARIANT parameters they do not own, and the implicit Drop would free memory still owned by the caller, causing double-frees and spurious Release calls on embedded values.

These types now implement Free instead, consistent with other Win32 types whose cleanup is context-dependent. Callers that require automatic cleanup should wrap values in Owned<T>:

{
  let v = Owned::new(VARIANT::from(my_bstr));
}
// VariantClear is called automatically

Fixes #3818

@riverar
Copy link
Copy Markdown
Collaborator Author

riverar commented Feb 10, 2026

I considered special-casing the [in] VARIANT parameters to emit *const VARIANT on x64/ARM64 and ManuallyDrop<VARIANT> on x86 (keying off DoNotReleaseAttribute) but this was abandoned because the metadata attribute isn't consistently applied and code-gen complexity would increase (i.e., #[cfg(target_arch = "x86")] all over the place).

@riverar
Copy link
Copy Markdown
Collaborator Author

riverar commented Feb 10, 2026

Manually verified #3818 continues to crash (cargo run --release) with 0.62.2 on Windows 26300.7760; switched to this branch, no longer crashes.

@kennykerr kennykerr merged commit ab01522 into master Feb 11, 2026
32 checks passed
@kennykerr kennykerr deleted the rafael/drop-variant-drop branch February 11, 2026 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

IUIAutomationPropertyChangedEventHandler - registering against array type property events causes heap corruption

2 participants