Skip to content

Classic-COM support: CoCreateInstance + vtable + natural codegen (interop/SMTC) - #65

Merged
leileizhang (lei9444) merged 33 commits into
microsoft:mainfrom
yeelam-gordon:feat/win32-com-tier1
Jul 30, 2026
Merged

Classic-COM support: CoCreateInstance + vtable + natural codegen (interop/SMTC)#65
leileizhang (lei9444) merged 33 commits into
microsoft:mainfrom
yeelam-gordon:feat/win32-com-tier1

Conversation

@yeelam-gordon

@yeelam-gordon Gordon Lam (yeelam-gordon) commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds classic (non‑WinRT) COM support to dynwinrt — both the runtime invocation path and generated, natural TypeScript/JavaScript wrappers — so app developers call Windows COM APIs (taskbar, Share, media controls, shell) with idiomatic JS and zero hand‑written IIDs, vtable indices, REFIID, void**, or CoCreateInstance boilerplate.

This is the classic‑COM half of taking dynwinrt beyond WinRT. It is a self‑contained vertical: runtime + codegen + tests + live E2E.

What you write (generated API)

// Taskbar progress — generated ITaskbarList3.js
import { ITaskbarList3 } from './generated/ITaskbarList3.js';
import { TBPFLAG } from './generated/TBPFLAG.js';

const taskbar = ITaskbarList3.create();           // CoCreateInstance, hidden
taskbar.hrInit();
taskbar.setProgressState(hwnd, TBPFLAG.TBPF_NORMAL);
taskbar.setProgressValue(hwnd, 40n, 100n);         // 40% — no IID, vtable, or void**

// Share sheet — generated IDataTransferManagerInterop.js (desktop *Interop shim)
import { IDataTransferManagerInterop } from './generated/IDataTransferManagerInterop.js';

const interop = IDataTransferManagerInterop.create();   // activationFactory + QI
const dtm = interop.getForWindow(hwnd);                 // -> DynWinRtValue bridge to DataTransferManager

No hand-written IIDs, REFIID, void**, vtable indices, or CoCreateInstance boilerplate — the generator emits it all.

Motivation

dynwinrt already dynamically projects WinRT (.winmd → libffi COM vtable calls). But many high‑value Windows surfaces are classic COM, not WinRT, and are unreachable from a WinRT‑only projection:

  • Taskbar progress / overlay / jump lists — ITaskbarList3, ICustomDestinationList
  • The desktop *Interop shims that WinRT features require from a windowed app — SMTC media controls (ISystemMediaTransportControlsInterop), Share (IDataTransferManagerInterop), Windows Hello (IUserConsentVerifierInterop), file‑picker parenting (IInitializeWithWindow)
  • Shell — IShellLink, IPropertyStore

Architecture

Windows.Win32.winmd ──▶ dynwinrt-codegen ──▶ natural .js + .d.ts
                        (base-aware slots)          │
                                                    ▼
      CoCreateInstance(CLSID,IID)  /  RoGetActivationFactory + QueryInterface
                                                    ▼
        libffi vtable dispatch ◀── napi (coCreateInstance, registerInterfaceUnknown, pointer)

Key design points:

  • WinRT = COM + IInspectable. The existing engine already does vtable + libffi dispatch; classic COM is the same minus IInspectable. The one structural change: user methods start at vtable slot 3 (after IUnknown) instead of slot 6 (after IInspectable). The generator reads each interface's real base from [NativeInheritance] and picks +3 or +6not hardcoded (verified: some *Interop interfaces are IUnknown‑rooted +3, e.g. DataTransferManager; SMTC's is IInspectable‑rooted +6).
  • Activation: CoCreateInstance(CLSID, IID) for coclasses; RoGetActivationFactory + QueryInterface to the interop IID for the *Interop pattern.
  • The *Interop HWND pattern HRESULT GetForWindow(HWND, REFIID, void**) is special‑cased to a natural getForWindow(hwnd): T — the REFIID is synthesized from the return type's default‑interface IID and the void** out‑pointer is hidden.
  • Portability / no silent breakage: the interop target IID is resolved from the winmd set passed to the generator (or the newest installed SDK Windows.winmd); codegen fails loud if it can't resolve one — it never emits a silently‑broken NULL riid.
  • HWND handoff: generated methods accept bigint | Buffer, so Electron's win.getNativeWindowHandle() (a Buffer) passes straight through.

Developer experience (sample usage)

Taskbar progress — pure classic COM (ITaskbarList3):

import { ITaskbarList3, TBPFLAG } from './generated/ITaskbarList3.js';

const taskbar = ITaskbarList3.create();          // hides CoCreateInstance(CLSID_TaskbarList) + QI
taskbar.hrInit();
const hwnd = win.getNativeWindowHandle();         // Electron Buffer (or a bigint)
taskbar.setProgressState(hwnd, TBPFLAG.TBPF_NORMAL);
taskbar.setProgressValue(hwnd, 30n, 100n);        // ULONGLONG → bigint; HRESULT failure → throws

The *Interop HWND shim — returns a live WinRT object (Share / SMTC):

import { DataTransferManager } from './generated/DataTransferManager.js';
const dtm = DataTransferManager.getForWindow(hwnd);   // REFIID + void** hidden
console.log(dtm.runtimeClassName);                    // "Windows.ApplicationModel.DataTransfer.DataTransferManager"

const smtc = SystemMediaTransportControls.getForWindow(hwnd);
smtc.isPlayEnabled  = true;                            // natural WinRT property
smtc.playbackStatus = MediaPlaybackStatus.Playing;

Generate a wrapper — partial, per class (never the whole 23 MB winmd):

dynwinrt-codegen generate --winmd Windows.Win32.winmd \
  --namespace Windows.Win32.UI.Shell --class-name ITaskbarList3 --output ./generated

Tests

Layer Coverage
Runtime — cargo test -p dynwinrt classic‑COM activation, +3 dispatch, live *Interop GetForWindow(hwnd) (DataTransferManager), IShellLinkW round‑trip → 83 passed + WinRT regression
Codegen — cargo test -p dynwinrt-codegen base‑aware vtable slots, CLSID resolution, interop REFIID/void** hiding, BOOLboolean, portable interop IID (fail‑loud), deterministic snapshots → 25+ tests
Live Node E2E taskbarlist.mjs, dtm.mjs, smtc.mjs — drive the generated wrappers against real Windows COM
WinRT regression unchanged: py 29/29, ts 28/28

An automated code‑review loop was run to convergence; every finding was addressed (COM refcount/lifetime, base‑chain root termination, memory‑safety of raw‑pointer inputs for typed vs untyped params, fail‑loud IID resolution).

Notes


⚠️ Open foundational issues (acknowledged — under rework)

leileizhang (@lei9444) re-reviewed head f8dc42d against main using real Windows.Win32.winmd metadata + live runtime probes and identified the following foundational ABI-correctness issues. They are acknowledged here for visibility and are being addressed before merge:

  1. Caller-owned byte buffers projected as scalar storage. e.g. IDiscRecorder::GetRecorderGUID(BYTE*, ULONG) emits .addInOut(DynCom.u8Type()) (storage for one byte), so a native write can overrun the generated storage (~39 interfaces).
  2. Namespace-mode COM emits wrong vtable slots. The namespace route does not preserve the full classic-COM inheritance chain; ITaskbarList3::SetProgressValue was emitted at slot 6 instead of 9 (invokes the wrong method). Single-class generation is correct.
  3. ISize/USize hardcoded as i64/u64. ABI mismatch on 32-bit Windows (~70 interfaces). (The earlier cdecl/stdcall concern was withdrawn after an i686 libffi probe passed.)
  4. Managed COM ownership can be duplicated via a raw bigint address. Exporting an owned pointer and re-adopting that address into another managed wrapper creates two owners of one COM reference; releasing both reproduced a Node access violation (0xC0000005).
  5. Typed-array pointer wrappers survive backing-buffer detachment. After the source ArrayBuffer is detached (byteLength -> 0), the retained native pointer stays non-null, so the wrapper can pass a stale pointer to native code.
  6. Required native parameters become optional. String-buffer rendering defaults later required parameters to null/zero (e.g. IExtractImage::GetLocation generates prgSize = 0).
  7. BSTR ownership + unsigned enums. IErrorInfo::GetDescription(BSTR*) returns via asPointerBigint without SysFreeString (leaks per call); FOFX_DONTDISPLAYLOCATIONS is emitted as -2147483648 instead of the unsigned 2147483648.
  8. COM-only output is not a consumable package. It emits interface/enum .js + .d.ts but no index/barrel or package manifest, so a standalone import of the generated output fails.

CI gap: the real Win32 metadata tests silently skip when DYNWINRT_WIN32_WINMD (or the expected local SDK metadata path) is unavailable, so the suite can pass without executing these cases.

See leileizhang (@lei9444)'s full review comment on this PR for reproduction details.

…shared napi plumbing

Reorganizes the Win32/COM work into a self-contained classic-vertical
that pairs the classic-COM runtime (call.rs RawPtr, classic_com.rs,
signature::define_from_iunknown), classic-COM/interop codegen
(codegen::com, main.rs --class-name COM path), and the shared napi
plumbing (coCreateInstance, registerInterfaceUnknown, pointer,
iidPointer, asPointerBigint, u64 Either fix, createTestHwnd) needed by
the ITaskbarList3 / DTM / SMTC E2Es.

Flat-Win32 pieces (flat_call.rs, codegen::flat, flatInvoke napi,
Apis/DllImport meta) are intentionally absent from this branch and are
layered back on top in reorg/flat-vertical.

The classic E2Es (taskbarlist.mjs, dtm.mjs, smtc.mjs) acquire a
process-owned HWND through a new tiny napi helper createTestHwnd()
(delegates to CreateWindowExW via windows-rs) instead of the
flatInvoke-based path used in the fully-integrated reference. A small
hwnd.mjs helper module encapsulates the call.

Gauntlet (green):
  - cargo test -p dynwinrt: 83 passed + 1 winrt_regression
  - cargo test -p dynwinrt-codegen: all suites green
  - napi build (release): OK
  - Node E2Es: taskbarlist.mjs / dtm.mjs / smtc.mjs PASS
  - tests\e2e_test.ps1 -SkipBuild: py 29/29, ts 28/28

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… enum`

Mirror the WinRT enum generator (render_enum_dts in
codegen/javascript/render/declarations.rs) so classic-COM enum .d.ts stays
consumable under TS isolatedModules and matches the JS Object.freeze runtime
shape.

Snapshot updated (tests/snapshots/itaskbarlist3/TBPFLAG.d.ts); the .js output
is unchanged so the taskbarlist E2E still works via TBPFLAG.TBPF_NORMAL member
access.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…t-param projection

- meta::parse_com_interface_from_index now tracks explicit termination at
  IUnknown or IInspectable and returns None if the base-chain walk exits
  without reaching either, instead of silently defaulting to IInspectable
  (base offset 6). This prevents wrong absolute vtable slots when a winmd
  has an unexpected inheritance shape or missing interface_impls.

- unwrap_return_js: for opaque Win32 handle out-params (HWND, PWSTR, ...)
  emit `.asPointerBigint()` instead of `.toI64()`. The runtime may
  produce WinRTValue::Object/RawPtr/Null when the handle's inner `Value`
  field is a void*-shaped type, and `.toI64()` panics on those variants
  (its fallback `.toNumber()` panics for non-numeric variants).
  `.asPointerBigint()` cleanly handles all three pointer representations.

All snapshots and E2Es unaffected: current interfaces have no [out] handle
params, and the two rooted tests (ITaskbarList3 → IUnknown, SMTC interop
→ IInspectable) still pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…om_index

Add an early guard that returns None unless the resolved TypeDef has
`TypeAttributes::Interface` set. Prevents WinRT runtime classes,
structs, enums, and delegates from being mis-parsed by walking their
`interface_impls()` and flattening a bogus method list — which could
have quietly routed `--class-name *Interop` runtime classes through
the classic-COM code path in `main.rs`.

Callers see `None` and fall through to the correct WinRT path.
All existing tests + node/py/ts E2Es unaffected (real interfaces still
have the Interface attribute set).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…path

- napi::create_test_hwnd() now caches the HWND in an AtomicUsize so
  repeated calls in a long-lived Node process (tests, REPL, Electron)
  don't accumulate window handles.

- codegen(com): handle typedef comments now say "Opaque Win32 handle or
  pointer newtype (e.g. HWND, PWSTR)" instead of just "Opaque Win32
  handle" — the aliases cover both handles and pointer newtypes like
  PWSTR/PCWSTR. Snapshots updated.

- meta::find_runtime_class_default_iid now collects all runtime-class
  matches for a simple name and refuses to pick when they resolve to
  distinct default-interface IIDs (cross-namespace collisions). Emits an
  explicit warning listing the candidates and returns None so callers
  fall through instead of silently generating interop wrappers with the
  wrong IID.

- DynWinRTValue.u64 number branch now takes f64 (not i64) so we can
  reject NaN, +/-Infinity, and fractional values explicitly. napi's
  previous i64 coercion silently truncated fractions and mis-handled
  non-finite inputs. Bigint path and MAX_SAFE_INTEGER bound unchanged.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…parsed

parse_com_interface_from_index previously logged a warning and continued
when parse_interface_with_offset returned None for a base interface.
That left slot_cursor unadvanced and produced a truncated flattened
method list, so the leaf interface's absolute vtable indices would be
off by however many base methods were missing. The debug_assert_eq!
below caught this in debug builds, but in release it was silently
compiled out — so codegen would emit wrappers that dispatch to the
wrong COM methods.

Now return None on any base-parse failure (with a warning naming both
the missing base and the leaf we're refusing to emit), so callers see
a clean skip rather than misgeneration.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- MetadataTable::register_interface and register_interface_iunknown now
  route through create_interface_method_table*(iid, base_slot) BEFORE
  checking the name cache. That call is already assertive on base_slot
  mismatch (arena.rs:131). Previously a first-time
  register_interface(name, iid) with base_slot=6 would let a later
  register_interface_iunknown(name, iid) — expecting base_slot=3 —
  silently reuse the WinRT-shaped vtable and dispatch to the wrong
  absolute slots. Now the mismatch panics loudly.

- meta::find_runtime_class_default_iid: replaced the `?` on the
  default-interface TypeDef lookup with a `let-else { continue }`.
  A missing/unreadable interface TypeDef for one candidate no longer
  aborts the whole search — other matching runtime classes (or other
  DefaultAttribute impls on the same class) can still resolve.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends dynwinrt beyond WinRT by adding a classic (IUnknown-rooted) COM invocation/codegen path, including base-aware vtable slot computation (+3 vs +6), CLSID-based activation via CoCreateInstance, and generation of “natural” JS/TS wrappers (including *Interop HWND shims that synthesize REFIID/void**).

Changes:

  • Add classic-COM metadata parsing and codegen routing (base-chain flattening, absolute vtable slots, CLSID discovery, interop IID resolution fallback to newest SDK Windows.winmd).
  • Extend the Rust runtime to support classic-COM base-slot indexing and raw-pointer passthrough for handle/void* parameters.
  • Add comprehensive tests/snapshots plus Node E2E fixtures for taskbar + interop (DTM/SMTC).

Reviewed changes

Copilot reviewed 30 out of 39 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tools/dynwinrt-codegen/tests/win32_com_test.rs TDD coverage for classic COM interface parsing, slots, CLSID resolution, and natural wrapper surfaces.
tools/dynwinrt-codegen/tests/win32_com_interop_test.rs TDD coverage for *Interop HWND pattern generation and portable IID resolution.
tools/dynwinrt-codegen/tests/snapshots/itaskbarlist3/TBPFLAG.js Snapshot fixture for generated enum JS output.
tools/dynwinrt-codegen/tests/snapshots/itaskbarlist3/TBPFLAG.d.ts Snapshot fixture for generated enum DTS output.
tools/dynwinrt-codegen/tests/snapshots/itaskbarlist3/ITaskbarList3.js Snapshot fixture for generated classic-COM wrapper JS output.
tools/dynwinrt-codegen/tests/snapshots/itaskbarlist3/ITaskbarList3.d.ts Snapshot fixture for generated classic-COM wrapper DTS output.
tools/dynwinrt-codegen/tests/snapshots/idatatransfermanagerinterop/IDataTransferManagerInterop.js Snapshot fixture for generated interop wrapper JS output.
tools/dynwinrt-codegen/tests/snapshots/idatatransfermanagerinterop/IDataTransferManagerInterop.d.ts Snapshot fixture for generated interop wrapper DTS output.
tools/dynwinrt-codegen/tests/snapshots/idatatransfermanagerinterop/DataTransferManager.js Snapshot fixture for generated projected companion JS output.
tools/dynwinrt-codegen/tests/snapshots/idatatransfermanagerinterop/DataTransferManager.d.ts Snapshot fixture for generated projected companion DTS output.
tools/dynwinrt-codegen/src/meta.rs Adds classic-COM interface parsing, base-aware vtable indexing, and portable runtime-class IID discovery helpers.
tools/dynwinrt-codegen/src/main.rs Routes --class-name between WinRT and classic-COM generation and emits standalone COM artifacts.
tools/dynwinrt-codegen/src/codegen/mod.rs Exposes the new classic-COM codegen module.
crates/dynwinrt/tests/winrt_regression.rs Adds WinRT regression harness to ensure WinRT behavior remains stable.
crates/dynwinrt/src/signature.rs Hardens raw-pointer inputs: only allow RawPtr for untyped Object/handle/void* params.
crates/dynwinrt/src/metadata_table/mod.rs Adds register_interface_iunknown and ensures base-slot conflicts fail loudly.
crates/dynwinrt/src/metadata_table/arena.rs Stores per-IID base_slot and uses it for method indexing/dedup.
crates/dynwinrt/src/lib.rs Exposes the new classic_com module from the crate.
crates/dynwinrt/src/classic_com.rs Implements COM apartment init + CoCreateInstance + classic COM dynamic call helpers/tests.
crates/dynwinrt/src/call.rs Extends scalar dispatch to support WinRTValue::RawPtr.
crates/dynwinrt/Cargo.toml Enables additional Windows metadata/features needed by new tests and classic COM scenarios.
bindings/js/src/lib.rs Adds N-API surface for classic COM registration, CoCreateInstance, pointer helpers, and related type aliases.
bindings/js/e2e/TBPFLAG.js Generated E2E enum fixture for taskbar tests.
bindings/js/e2e/taskbarlist.mjs Node E2E exercising generated classic-COM taskbar wrapper.
bindings/js/e2e/SystemMediaTransportControls.js Generated E2E projected companion for SMTC interop scenario.
bindings/js/e2e/SystemMediaTransportControls.d.ts DTS for SMTC projected companion fixture.
bindings/js/e2e/smtc.mjs Node E2E for IInspectable-rooted SMTC interop + meaningful member access via projected wrapper.
bindings/js/e2e/package.json Marks E2E directory as ESM module scope.
bindings/js/e2e/ITaskbarList3.js Generated E2E classic-COM wrapper fixture (taskbar).
bindings/js/e2e/ISystemMediaTransportControlsInterop.js Generated E2E interop wrapper fixture (SMTC).
bindings/js/e2e/ISystemMediaTransportControlsInterop.d.ts DTS for SMTC interop wrapper fixture.
bindings/js/e2e/IDataTransferManagerInterop.js Generated E2E interop wrapper fixture (DTM).
bindings/js/e2e/IDataTransferManagerInterop.d.ts DTS for DTM interop wrapper fixture.
bindings/js/e2e/hwnd.mjs Node helper that acquires a process-owned HWND via N-API helper.
bindings/js/e2e/dtm.mjs Node E2E for IUnknown-rooted DTM interop + runtimeClassName liveness proof.
bindings/js/e2e/DataTransferManager.js Generated E2E projected companion fixture (DTM).
bindings/js/e2e/DataTransferManager.d.ts DTS for DTM projected companion fixture.
.gitignore Ignores bulky generated E2E projection fixtures to keep PRs reviewable.

Comment thread bindings/js/src/lib.rs Outdated
Gordon Lam (yeelam-gordon) and others added 10 commits July 23, 2026 09:19
…+ COM pointer adoption

Fixes 4 gaps surfaced by a systematic Windows.Win32 winmd exploration sweep:
- C4: add napi DynWinRtType.u16Type() (+ i16Type/u8Type/f32Type/f64Type aliases).
- C1: [in] HRESULT params project as number / i32Type() / DynWinRtValue.i32(hr).
- C2: caller-owned [out] PWSTR + cch string buffers (IShellLinkW.GetPath/
      GetDescription) generate a real wrapper via ParamDirection::OutStringBuffer;
      narrow detector (direct PWSTR/PSTR + adjacent char-count; PWSTR* not matched;
      cb byte-counts excluded for PWSTR; PSTR fails loud). Previously crashed.
- C3: adoptComPointer(ptr, iid?) adopts an AddRef-owned returned COM pointer via
      IUnknown::from_raw (no extra AddRef) + optional QI-validate; codegen wraps
      directly-named TypeMeta::Interface out-params as typed wrappers.

Tests: TDD unit tests each; refcount-correct native adoption test;
e2e/shelllink-buffer.mjs proves SetPath/GetPath + SetDescription/GetDescription
round-trips on live classic COM.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…Value

Copilot review caught a real runtime bug: classic-COM codegen emitted
DynWinRtValue.u16Value(...)/i16Value(...) for [in] u16/i16 params, but the napi
binding exports u16()/i16() (only bool/i8/u8 use the *Value suffix). Any method
with a u16/i16 input (e.g. IShellLinkW.SetHotkey) threw a TypeError at runtime.

Fix the two wrong match arms in com.rs wrap_arg_js to the ctor names that exist.
Adds a codegen regression test (u16 param must wrap via DynWinRtValue.u16, never
u16Value/i16Value) and a live setHotkey no-throw assertion in shelllink-buffer.mjs.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…gen)

Copilot review found metadata_table/type_kind.rs IMAP_VIEW was
e9bdaaf0-cbf6-4c39-de49-316b34326a17, but the code generator uses the correct
WinRT IMapView\2 PIID e480ce40-a338-4ada-adcf-272272e48cb9 in four places
(javascript/{method,project/mod,signature}.rs, python/collections.rs).

MetadataTable::map_iids() feeds IMAP_VIEW into compute_parameterized_iid
(metadata_table/mod.rs:378), so the mismatch produced wrong IMapView<K,V> IIDs at
runtime and would break map-view projections on QueryInterface. Latent (no test
exercised IMapView<K,V>). Aligns the runtime constant to the canonical PIID and
adds a regression guard test.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…t-values

Copilot review (PR #1): methods with a scalar [out] pointer (IShellLinkW.GetShowCmd
[out] int*, GetHotkey [out] WORD*, GetIconLocation [out] int* piIcon) were generated
as .addOut(pointer()) and returned the raw DynWinRtValue with a bogus COM-pointer
adoption TODO — an unusable result.

Root cause: meta.rs collapsed scalar [out] pointers to TypeMeta::Object, losing the
pointee type. Preserve the scalar pointee (GetShowCmd -> SHOW_WINDOW_CMD enum(I32),
GetHotkey -> U16, GetIconLocation.piIcon -> I32) and project such out-params as
.addOut(<scalar>Type()) returning a JS number via .toNumber(). Interface out-params
(pointer()+_fromNative) and string out-buffers (OutStringBuffer) are unaffected.

Adds codegen regression tests and a live getShowCmd/getHotkey round-trip in
shelllink-buffer.mjs.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move classic-COM metadata, ABI signatures, pointer ownership, codegen, and JS bindings into parallel COM-specific layers while preserving existing WinRT models and APIs.
…nsferManager substring

The interop return-type assertion matched the bare substring "DataTransferManager",
which is always present as part of the interop class name `IDataTransferManagerInterop`.
That made it a false positive: it passed regardless of the real return type and its
message ("must project the return type as DataTransferManager") contradicted the actual
design, which returns the explicit `DynWinRtValue` bridge (no synthesized WinRT
runtime-class projection). Assert the real contract instead:
`getForWindow(appWindow: HWND): DynWinRtValue;`.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The fallback error hardcoded "CoTaskMem-allocated", but take_raw_pointer serves
multiple pointer kinds (description = "COM interface", "wide-string",
"ANSI-string", "CoTaskMem allocation"). The claim was inaccurate for 3 of 4
callers. The `description` parameter already conveys the kind, so drop the
misleading qualifier: "Expected a {description} raw pointer".

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… assertions

1. Address review feedback: WIN32_WINMD was hardcoded to a developer-local path
   (C:\s\win32metadata\Windows.Win32.winmd), so the whole classic-COM test suite
   silently self-skipped on CI and other machines. Replace the const with a
   win32_winmd() helper that honors the DYNWINRT_WIN32_WINMD environment variable
   and falls back to the local path. Applied to win32_com_test.rs and
   win32_com_interop_test.rs.

2. Because those tests never ran in CI, two assertions went stale after the
   "isolate classic COM from WinRT" refactor and were failing locally:
     - shellitem_getdisplayname_...: expected DynWinRtMethodSig/DynWinRtType.i32Type()
     - u16_input_param_...: expected DynWinRtValue.u16(wHotkey)
   Classic-COM codegen now emits DynComMethodSig / DynCom.i32Type() / DynCom.u16(...).
   Update the expectations (intent unchanged: callee-allocated addOut(pointer), and
   u16 wrapped via the existing ctor). Full dynwinrt-codegen suite now green.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…-back iid_pointer

Two classic-COM memory fixes surfaced by the pointer-lifecycle audit:

#2 (Medium, double-release hazard): DynCom.pointer() accepted an existing
DynWinRtValue and, for Object values, returned a borrowed raw COM pointer
owner-backed by a clone. That raw pointer is indistinguishable from an owned
+1 pointer to adoptComPointer(), enabling a double-release. Align tier1 with
tier2: reject all DynWinRtValue inputs; callers pass raw pointer bits,
Buffer/Uint8Array, or null. Generated COM code only ever passes HWND/buffer/
PIDL values to pointer(), never objects, so nothing breaks. Adds
e2e/pointer-reject-object.mjs regression.

#4 (Low, unbounded leak): iid_pointer boxed one GUID per distinct GUID into a
static HashMap and never freed it. Replace with an owner-backed
NativePointerOwner::Guid(Box<GUID>) that frees on drop/GC. The REFIID is only
read during the synchronous COM call and the JS temporary outlives it.
Classic-COM gauntlet (taskbarlist, dtm, smtc, shelllink, hwnd) all pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Gordon Lam (yeelam-gordon) added a commit to yeelam-gordon/dynwinrt that referenced this pull request Jul 24, 2026
Brings the latest classic-COM fixes into the combined branch:
- #2 pointer() rejects DynWinRtValue inputs (double-release hazard)
- #4 iid_pointer owner-backed GUID (no static leak)
- env-overridable DYNWINRT_WIN32_WINMD test path + stale-assertion fixes
- take_raw_pointer ownership-neutral message; interop return-type assertion

Conflict resolved in win32_com_test.rs (comment wording only; both sides
already assert the DynCom.u16(...) contract).
Classic COM handle-value newtypes like HWND were projected as bigint | Buffer, but DynCom.pointer(Buffer) passes the Buffer's address instead of the handle bits it contains. Emit handle values as bigint | number while keeping NUL-terminated string pointer aliases such as PWSTR as bigint | Buffer.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Backfills the missing test for the #4 memory fix (commit 50193b4): asserts
iid_pointer returns an owner-backed DynWinRtValue (NativePointerOwner::Guid, so
the boxed GUID frees on drop) holding the correct GUID bytes, and that two
concurrently-live calls for the same GUID allocate distinct boxes (no shared
static cache). Verified this FAILS against the pre-fix static-cache-leak impl
(the owner-backed assertion fails) and passes after.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Resolve the JS binding conflict by preserving Win32 COM pointer ownership alongside the shared async Promise and WinUI dispatch paths. Integrate GUID value returns with the merged ABI model.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 318436f7-b069-4eeb-a0bd-b9ff676e3194
@lei9444

leileizhang (lei9444) commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

I re-reviewed and experimentally verified the current PR head (f8dc42d993fdb3ebaf1fb4806a523d3b13145014) against main using real Windows.Win32.winmd metadata and live runtime probes.

The principles I used for this review are:

  1. Existing WinRT metadata, generated bindings, and DynWinRt* APIs must remain unchanged.
  2. Classic COM should be a parallel DynCom*/COM-metadata layer and share only private ABI, libffi, and vtable-dispatch infrastructure with WinRT.
  3. The projection must preserve the native ABI exactly: interface inheritance and slots, return convention, pointer width/depth, and parameter direction.
  4. Pointer ownership, allocator, provenance, and backing-storage lifetime must be explicit; an owned pointer must not be exportable and silently re-adoptable as a second owner.
  5. Unsupported ABI shapes must fail closed during generation instead of producing plausible but unsafe bindings.

The following issues are reproducible:

  1. Caller-owned byte buffers are projected as scalar storage. A scan of real metadata found 39 affected interfaces. For example, IDiscRecorder::GetRecorderGUID(BYTE*, ULONG) generates .addInOut(DynCom.u8Type()), which allocates storage for one byte rather than the caller-sized buffer described by the count parameter. A native write can therefore overrun the generated storage.

  2. Namespace generation uses incorrect Classic COM vtable slots. In namespace mode, ITaskbarList3::SetProgressValue was emitted at slot 6 instead of slot 9 because the route does not preserve the complete Classic COM inheritance chain. This invokes the wrong method.

  3. The x86 calling-convention concern was not reproduced, but the x86 pointer-width bug was. A three-argument extern "system" libffi probe passed on the i686 target, so I am withdrawing the earlier cdecl/stdcall claim. However, ISize/USize are generated as fixed i64/u64; about 70 interfaces are affected. For example, IApartmentCallback parameters typed as usize generate u64Type(), which is an ABI mismatch on 32-bit Windows.

  4. Managed COM ownership can be duplicated through a raw bigint address. Exporting an owned pointer address and adopting that address into another managed wrapper creates two owners of the same COM reference. Releasing both wrappers reproduced a Node access violation (0xC0000005).

  5. Typed-array pointer wrappers survive backing-buffer detachment. After transferring/detaching the source ArrayBuffer, its byteLength became 0, but the retained native pointer stayed unchanged and non-null. The wrapper can therefore pass a stale pointer to native code.

  6. Required native parameters become optional. String-buffer rendering defaults later required parameters to null/zero. For example, generated IExtractImage::GetLocation code has prgSize = 0, allowing a required output pointer to be omitted and forwarded as null.

  7. BSTR ownership and unsigned enums are incorrect. IErrorInfo::GetDescription(BSTR*) returns the allocation as asPointerBigint without calling SysFreeString, so each successful call leaks. Also, FOFX_DONTDISPLAYLOCATIONS is emitted as -2147483648 instead of the unsigned value 2147483648.

  8. COM-only output is not a consumable package. It contains interface/enum .js and .d.ts files but no generated index/barrel or package manifest. A standalone import of the generated output fails.

There is also a CI coverage gap: the real Win32 metadata tests silently skip when DYNWINRT_WIN32_WINMD (or the expected local SDK metadata path) is unavailable, so the test suite can pass without executing these cases.

Gordon Lam (yeelam-gordon) and others added 10 commits July 28, 2026 18:24
…anual unwrap)

Classic-COM handle-value params (HWND/HANDLE/HKEY…) now accept an Electron/Node
Buffer/Uint8Array (e.g. BrowserWindow.getNativeWindowHandle()) directly, read as
the handle VALUE, in addition to bigint|number. Removes the .readBigUInt64LE(0)
tax for Electron callers.

- wrap_arg_js: handle-VALUE args wrap via a generated `_handleArg(x)` helper
  (Buffer/Uint8Array → little-endian pointer value; bigint|number pass-through).
  String-pointer handles (PWSTR) keep address semantics — NOT wrapped.
- render_js: emit the inline `_handleArg` helper only when an interface has a
  handle-value input (new `uses_handle_value_input` predicate).
- .d.ts handle typedef: `bigint | number | Buffer | Uint8Array` (+ reworded doc).
- Pure codegen: emits `pointer(_handleArg(x))` = pointer(bigint) — no napi change.
- Tests: flipped the rejects-buffer test to accepts-buffer (proven fail-before:
  reverting the wrap fails "handle arg must be unwrapped"); updated param_type_mapping
  assertion; regenerated ITaskbarList3 + IDataTransferManagerInterop snapshots.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep Classic COM isolated from WinRT while fixing pointer ownership, target-width ABI types, buffer validation, BSTR lifetime, unsigned enum projection, required parameters, and package generation.

Move Classic COM E2E coverage into the unified test pipeline, require real Win32 metadata in CI, and remove checked-in generated fixtures.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 41a826d4-b04a-4932-b1b8-31173fc5ab40
Keep the package root WinRT-only and expose DynCom APIs through @microsoft/dynwinrt/com over the same native binary. Generate typed CJS facades and wire them through codegen, CI, release, samples, and E2E.

Expand stock-Windows coverage for IPersistFile, IMalloc, IStream, IFileOperation, IFileOpenDialog, and IWICImagingFactory, including ownership and pointer-width ABI paths.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 41a826d4-b04a-4932-b1b8-31173fc5ab40
Quantify the Windows.Win32 metadata surface and public-code frequency sample, document supported and unsupported ABI shapes, summarize implemented fixes, and define ownership and future type-system priorities.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 41a826d4-b04a-4932-b1b8-31173fc5ab40
Merge 9ecbdaf while replacing its generated broad _handleArg heuristic with centralized DynCom.handleValue conversion. Accept exact pointer-width Electron HWND buffers, preserve data-pointer Buffer address semantics, retain numeric output aliases, and cover detached/wrong typed arrays, PSID, InOut, x86, and live HWND calls.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 41a826d4-b04a-4932-b1b8-31173fc5ab40
Define type-and-contract-first modeling, runtime/codegen/renderer responsibilities, Buffer and ownership semantics, fail-closed requirements, validation expectations, and the invariant that Classic COM changes must not alter WinRT behavior.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 41a826d4-b04a-4932-b1b8-31173fc5ab40
Keep WinRT signatures and metadata isolated while Classic COM owns its method registry and lowers through a private native-call backend. Add exact struct validation, preserve ABI-compatible array projections, and document the runtime boundaries.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 41a826d4-b04a-4932-b1b8-31173fc5ab40
Project HSTRING with ownership, require resolved interface metadata, and preserve semantic HRESULT values. Close unsafe pointer fallbacks for unresolved, compound, callback, array, and dynamic-IID shapes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 41a826d4-b04a-4932-b1b8-31173fc5ab40
Move WinRT generators under a dedicated domain and lower Classic COM metadata into validated ComType and ProjectedComMethod IR before rendering. Remove pointer fallbacks, encode ownership and return semantics explicitly, and retain current main WinRT generation behavior.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 41a826d4-b04a-4932-b1b8-31173fc5ab40
Resolve the observable-vector test conflict by retaining the runtime asVector assertion while accepting the latest main dependency and documentation updates.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 41a826d4-b04a-4932-b1b8-31173fc5ab40
@lei9444
leileizhang (lei9444) merged commit 181af5a into microsoft:main Jul 30, 2026
5 checks passed
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.

4 participants