From d132acb94a3974628a8a55e29e95266084dd0caa Mon Sep 17 00:00:00 2001 From: wisper-sabrina Date: Wed, 17 Jun 2026 09:41:54 +0200 Subject: [PATCH 1/5] The asynchronous WUA methods (BeginSearch, BeginDownload, BeginInstall) require a non-NULL IUnknown* callback argument. Passing NULL (VT_NULL) makes them fail with DISP_E_TYPEMISMATCH (0x80020005), so I fixed the functions --- iupdatedownloader.go | 2 +- iupdateinstaller.go | 4 +- iupdatesearcher.go | 2 +- olecallback.go | 139 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 143 insertions(+), 4 deletions(-) create mode 100644 olecallback.go diff --git a/iupdatedownloader.go b/iupdatedownloader.go index 67a1030..6d58c2e 100644 --- a/iupdatedownloader.go +++ b/iupdatedownloader.go @@ -88,7 +88,7 @@ func (iUpdateDownloader *IUpdateDownloader) BeginDownload(updates []*IUpdate) (* return nil, err } - jobDisp, err := toIDispatchErr(oleutil.CallMethod(iUpdateDownloader.disp, "BeginDownload", nil, nil, nil)) + jobDisp, err := toIDispatchErr(oleutil.CallMethod(iUpdateDownloader.disp, "BeginDownload", newNoopDispatch(), newNoopDispatch(), nil)) if err != nil { return nil, err } diff --git a/iupdateinstaller.go b/iupdateinstaller.go index ef2741d..c026a38 100644 --- a/iupdateinstaller.go +++ b/iupdateinstaller.go @@ -155,7 +155,7 @@ func (iUpdateInstaller *IUpdateInstaller) BeginInstall(updates []*IUpdate) (*IIn return nil, err } - jobDisp, err := toIDispatchErr(oleutil.CallMethod(iUpdateInstaller.disp, "BeginInstall", nil, nil, nil)) + jobDisp, err := toIDispatchErr(oleutil.CallMethod(iUpdateInstaller.disp, "BeginInstall", newNoopDispatch(), newNoopDispatch(), nil)) if err != nil { return nil, err } @@ -183,7 +183,7 @@ func (iUpdateInstaller *IUpdateInstaller) BeginUninstall(updates []*IUpdate) (*I return nil, err } - jobDisp, err := toIDispatchErr(oleutil.CallMethod(iUpdateInstaller.disp, "BeginUninstall", nil, nil, nil)) + jobDisp, err := toIDispatchErr(oleutil.CallMethod(iUpdateInstaller.disp, "BeginUninstall", newNoopDispatch(), newNoopDispatch(), nil)) if err != nil { return nil, err } diff --git a/iupdatesearcher.go b/iupdatesearcher.go index a8ac890..f9b64c2 100644 --- a/iupdatesearcher.go +++ b/iupdatesearcher.go @@ -103,7 +103,7 @@ func (iUpdateSearcher *IUpdateSearcher) QueryHistoryAll() ([]*IUpdateHistoryEntr // BeginSearch begins an asynchronous search for updates. // https://learn.microsoft.com/en-us/windows/win32/api/wuapi/nf-wuapi-iupdatesearcher-beginsearch func (iUpdateSearcher *IUpdateSearcher) BeginSearch(criteria string) (*ISearchJob, error) { - jobDisp, err := toIDispatchErr(oleutil.CallMethod(iUpdateSearcher.disp, "BeginSearch", criteria, nil, nil)) + jobDisp, err := toIDispatchErr(oleutil.CallMethod(iUpdateSearcher.disp, "BeginSearch", criteria, newNoopDispatch(), nil)) if err != nil { return nil, err } diff --git a/olecallback.go b/olecallback.go new file mode 100644 index 0000000..192773c --- /dev/null +++ b/olecallback.go @@ -0,0 +1,139 @@ +/* +Copyright 2022 Zheng Dayu +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +//go:build windows + +package windowsupdate + +import ( + "sync" + "syscall" + "unsafe" + + "github.com/go-ole/go-ole" +) + +// The asynchronous WUA methods (BeginSearch, BeginDownload, BeginInstall) require +// a non-NULL IUnknown* callback argument. Passing NULL (VT_NULL) makes them fail +// with DISP_E_TYPEMISMATCH (0x80020005). newNoopDispatch returns a minimal +// IDispatch whose Invoke does nothing (returns S_OK): completion is obtained +// through the blocking EndXxx methods and progress through IXxxJob.GetProgress(). +// +// The handler signatures are 100% uintptr because that is required by +// syscall.NewCallback. + +type noopCallbackVtbl struct { + pQueryInterface uintptr + pAddRef uintptr + pRelease uintptr + pGetTypeInfoCount uintptr + pGetTypeInfo uintptr + pGetIDsOfNames uintptr + pInvoke uintptr +} + +// noopCallback : lpVtbl MUST be the first field (the COM interface pointer points +// to it). +type noopCallback struct { + lpVtbl *noopCallbackVtbl + ref int32 +} + +// HRESULT values as uintptr (only the low 32 bits are significant). +const ( + hrSOK = uintptr(0x00000000) + hrENoInterface = uintptr(0x80004002) + hrENotImpl = uintptr(0x80004001) +) + +func ncQueryInterface(this, iid, ppvObject uintptr) uintptr { + guid := (*ole.GUID)(unsafe.Pointer(iid)) + out := (*uintptr)(unsafe.Pointer(ppvObject)) + if ole.IsEqualGUID(guid, ole.IID_IUnknown) || ole.IsEqualGUID(guid, ole.IID_IDispatch) { + p := (*noopCallback)(unsafe.Pointer(this)) + p.ref++ + if out != nil { + *out = this + } + return hrSOK + } + if out != nil { + *out = 0 + } + return hrENoInterface +} + +func ncAddRef(this uintptr) uintptr { + p := (*noopCallback)(unsafe.Pointer(this)) + p.ref++ + return uintptr(uint32(p.ref)) +} + +func ncRelease(this uintptr) uintptr { + p := (*noopCallback)(unsafe.Pointer(this)) + p.ref-- + return uintptr(uint32(p.ref)) +} + +func ncGetTypeInfoCount(this, pctinfo uintptr) uintptr { + if pctinfo != 0 { + *(*uint32)(unsafe.Pointer(pctinfo)) = 0 + } + return hrSOK +} + +func ncGetTypeInfo(this, iTInfo, lcid, ppTInfo uintptr) uintptr { + return hrENotImpl +} + +func ncGetIDsOfNames(this, riid, rgszNames, cNames, lcid, rgDispId uintptr) uintptr { + return hrENotImpl +} + +// ncInvoke : no-op body. WUA calls DISPID 0 on progress/completion; we ignore it +// and return S_OK. Completion is detected through EndXxx (blocking). +func ncInvoke(this, dispIdMember, riid, lcid, wFlags, pDispParams, pVarResult, pExcepInfo, puArgErr uintptr) uintptr { + return hrSOK +} + +var ( + noopVtbl *noopCallbackVtbl + noopOnce sync.Once + keepAliveMu sync.Mutex + keepAlive []*noopCallback // pin the objects so the GC does not collect them while WUA holds them +) + +func getNoopVtbl() *noopCallbackVtbl { + noopOnce.Do(func() { + noopVtbl = &noopCallbackVtbl{ + pQueryInterface: syscall.NewCallback(ncQueryInterface), + pAddRef: syscall.NewCallback(ncAddRef), + pRelease: syscall.NewCallback(ncRelease), + pGetTypeInfoCount: syscall.NewCallback(ncGetTypeInfoCount), + pGetTypeInfo: syscall.NewCallback(ncGetTypeInfo), + pGetIDsOfNames: syscall.NewCallback(ncGetIDsOfNames), + pInvoke: syscall.NewCallback(ncInvoke), + } + }) + return noopVtbl +} + +// newNoopDispatch creates a minimal IDispatch usable as a WUA callback. +// The object is pinned (keepAlive) so it is not collected while WUA holds it. +func newNoopDispatch() *ole.IDispatch { + cb := &noopCallback{lpVtbl: getNoopVtbl(), ref: 1} + keepAliveMu.Lock() + keepAlive = append(keepAlive, cb) + keepAliveMu.Unlock() + return (*ole.IDispatch)(unsafe.Pointer(cb)) +} From fd56b118d9002522c3bd6a7a2d66796a42b3c44e Mon Sep 17 00:00:00 2001 From: wisper-sabrina Date: Tue, 23 Jun 2026 09:04:40 +0200 Subject: [PATCH 2/5] fix callback Co-authored-by: Copilot --- idownloadjob.go | 9 +++ iinstallationjob.go | 9 +++ iupdatedownloader.go | 2 +- iupdateinstaller.go | 4 +- iupdatesearcher.go | 2 +- olecallback.go | 153 +++++++++++++++++++++++++++++-------------- olecallback_test.go | 75 +++++++++++++++++++++ 7 files changed, 202 insertions(+), 52 deletions(-) create mode 100644 olecallback_test.go diff --git a/idownloadjob.go b/idownloadjob.go index fdf4509..c819d3e 100644 --- a/idownloadjob.go +++ b/idownloadjob.go @@ -66,6 +66,15 @@ func (j *IDownloadJob) RequestAbort() error { return err } +// GetIsCompleted reads the live IsCompleted property of the job. Unlike the +// IsCompleted struct field (captured once at construction time, hence always +// false right after BeginDownload), this reflects the current state and is the +// authoritative completion signal for the async download. +// https://learn.microsoft.com/en-us/windows/win32/api/wuapi/nf-wuapi-idownloadjob-get_iscompleted +func (j *IDownloadJob) GetIsCompleted() (bool, error) { + return toBoolErr(oleutil.GetProperty(j.disp, "IsCompleted")) +} + // GetProgress returns the current progress of the download. // https://learn.microsoft.com/en-us/windows/win32/api/wuapi/nf-wuapi-idownloadjob-getprogress func (j *IDownloadJob) GetProgress() (*IDownloadProgress, error) { diff --git a/iinstallationjob.go b/iinstallationjob.go index 9c27f8c..b40c948 100644 --- a/iinstallationjob.go +++ b/iinstallationjob.go @@ -66,6 +66,15 @@ func (j *IInstallationJob) RequestAbort() error { return err } +// GetIsCompleted reads the live IsCompleted property of the job. Unlike the +// IsCompleted struct field (captured once at construction time, hence always +// false right after BeginInstall), this reflects the current state and is the +// authoritative completion signal for the async installation. +// https://learn.microsoft.com/en-us/windows/win32/api/wuapi/nf-wuapi-iinstallationjob-get_iscompleted +func (j *IInstallationJob) GetIsCompleted() (bool, error) { + return toBoolErr(oleutil.GetProperty(j.disp, "IsCompleted")) +} + // GetProgress returns the current progress of the installation. // https://learn.microsoft.com/en-us/windows/win32/api/wuapi/nf-wuapi-iinstallationjob-getprogress func (j *IInstallationJob) GetProgress() (*IInstallationProgress, error) { diff --git a/iupdatedownloader.go b/iupdatedownloader.go index 6d58c2e..b0a2d8a 100644 --- a/iupdatedownloader.go +++ b/iupdatedownloader.go @@ -88,7 +88,7 @@ func (iUpdateDownloader *IUpdateDownloader) BeginDownload(updates []*IUpdate) (* return nil, err } - jobDisp, err := toIDispatchErr(oleutil.CallMethod(iUpdateDownloader.disp, "BeginDownload", newNoopDispatch(), newNoopDispatch(), nil)) + jobDisp, err := toIDispatchErr(oleutil.CallMethod(iUpdateDownloader.disp, "BeginDownload", newNoopCallback(), newNoopCallback(), nil)) if err != nil { return nil, err } diff --git a/iupdateinstaller.go b/iupdateinstaller.go index c026a38..259994b 100644 --- a/iupdateinstaller.go +++ b/iupdateinstaller.go @@ -155,7 +155,7 @@ func (iUpdateInstaller *IUpdateInstaller) BeginInstall(updates []*IUpdate) (*IIn return nil, err } - jobDisp, err := toIDispatchErr(oleutil.CallMethod(iUpdateInstaller.disp, "BeginInstall", newNoopDispatch(), newNoopDispatch(), nil)) + jobDisp, err := toIDispatchErr(oleutil.CallMethod(iUpdateInstaller.disp, "BeginInstall", newNoopCallback(), newNoopCallback(), nil)) if err != nil { return nil, err } @@ -183,7 +183,7 @@ func (iUpdateInstaller *IUpdateInstaller) BeginUninstall(updates []*IUpdate) (*I return nil, err } - jobDisp, err := toIDispatchErr(oleutil.CallMethod(iUpdateInstaller.disp, "BeginUninstall", newNoopDispatch(), newNoopDispatch(), nil)) + jobDisp, err := toIDispatchErr(oleutil.CallMethod(iUpdateInstaller.disp, "BeginUninstall", newNoopCallback(), newNoopCallback(), nil)) if err != nil { return nil, err } diff --git a/iupdatesearcher.go b/iupdatesearcher.go index f9b64c2..a4b92a7 100644 --- a/iupdatesearcher.go +++ b/iupdatesearcher.go @@ -103,7 +103,7 @@ func (iUpdateSearcher *IUpdateSearcher) QueryHistoryAll() ([]*IUpdateHistoryEntr // BeginSearch begins an asynchronous search for updates. // https://learn.microsoft.com/en-us/windows/win32/api/wuapi/nf-wuapi-iupdatesearcher-beginsearch func (iUpdateSearcher *IUpdateSearcher) BeginSearch(criteria string) (*ISearchJob, error) { - jobDisp, err := toIDispatchErr(oleutil.CallMethod(iUpdateSearcher.disp, "BeginSearch", criteria, newNoopDispatch(), nil)) + jobDisp, err := toIDispatchErr(oleutil.CallMethod(iUpdateSearcher.disp, "BeginSearch", criteria, newNoopCallback(), nil)) if err != nil { return nil, err } diff --git a/olecallback.go b/olecallback.go index 192773c..d24754c 100644 --- a/olecallback.go +++ b/olecallback.go @@ -1,3 +1,5 @@ +//go:build windows + /* Copyright 2022 Zheng Dayu Licensed under the Apache License, Version 2.0 (the "License"); @@ -11,12 +13,11 @@ See the License for the specific language governing permissions and limitations under the License. */ -//go:build windows - package windowsupdate import ( "sync" + "sync/atomic" "syscall" "unsafe" @@ -24,22 +25,40 @@ import ( ) // The asynchronous WUA methods (BeginSearch, BeginDownload, BeginInstall) require -// a non-NULL IUnknown* callback argument. Passing NULL (VT_NULL) makes them fail -// with DISP_E_TYPEMISMATCH (0x80020005). newNoopDispatch returns a minimal -// IDispatch whose Invoke does nothing (returns S_OK): completion is obtained -// through the blocking EndXxx methods and progress through IXxxJob.GetProgress(). +// a non-NULL callback argument. Passing NULL (VT_NULL) makes them fail with +// DISP_E_TYPEMISMATCH (0x80020005). +// +// IMPORTANT: those parameters are NOT IDispatch. They are custom IUnknown-derived +// interfaces (ISearchCompletedCallback, IDownloadProgressChangedCallback, +// IDownloadCompletedCallback, IInstallationProgressChangedCallback, +// IInstallationCompletedCallback). Each declares exactly one method, +// Invoke(IXxxJob*, IXxxCallbackArgs*), located at vtable slot 3 (right after +// IUnknown's QueryInterface/AddRef/Release). They all share the same layout, so +// one 4-entry vtable serves as a universal no-op callback. +// +// A previous version built an IDispatch vtable (7 entries). When WUA invoked the +// completion callback it called slot 3 — which in an IDispatch layout is +// GetTypeInfoCount(this, pctinfo) — passing the job pointer as pctinfo. The body +// wrote 0 through that pointer, corrupting the job's vtable pointer and crashing +// the process (the service then restarts). +// +// AGILITY: WUA runs the asynchronous operation on its own worker thread and +// invokes the callback from there. Our STA session lives on a different COM +// apartment, so COM must marshal the callback across apartments. A raw Go vtable +// object has no marshaler, which made BeginXxx fail with DISP_E_EXCEPTION +// ("Une exception s'est produite"). To fix that we aggregate the COM +// Free-Threaded Marshaler (FTM): QueryInterface(IID_IMarshal) is delegated to the +// FTM, which makes the object agile (callable directly from any apartment, no +// proxy). BeginXxx then succeeds and progress is read by polling IXxxJob.GetProgress(). // // The handler signatures are 100% uintptr because that is required by // syscall.NewCallback. type noopCallbackVtbl struct { - pQueryInterface uintptr - pAddRef uintptr - pRelease uintptr - pGetTypeInfoCount uintptr - pGetTypeInfo uintptr - pGetIDsOfNames uintptr - pInvoke uintptr + pQueryInterface uintptr + pAddRef uintptr + pRelease uintptr + pInvoke uintptr // slot 3: Invoke for every WUA *Completed/*ProgressChanged callback } // noopCallback : lpVtbl MUST be the first field (the COM interface pointer points @@ -47,26 +66,52 @@ type noopCallbackVtbl struct { type noopCallback struct { lpVtbl *noopCallbackVtbl ref int32 + ftm uintptr // IUnknown* of the aggregated free-threaded marshaler (0 if unavailable) } // HRESULT values as uintptr (only the low 32 bits are significant). const ( hrSOK = uintptr(0x00000000) hrENoInterface = uintptr(0x80004002) - hrENotImpl = uintptr(0x80004001) +) + +// WUA callback interface IIDs and IID_IMarshal. The callback IIDs are a +// belt-and-suspenders allowlist: even if WUA only ever queries IUnknown (the +// declared parameter type) and uses the pointer directly, accepting the specific +// IIDs is harmless because they all share our vtable layout. +var ( + iidIMarshal = ole.NewGUID("{00000003-0000-0000-C000-000000000046}") + iidSearchCompleted = ole.NewGUID("{88AEE058-D4B0-4725-A2F1-814A67AE964C}") + iidDownloadProgress = ole.NewGUID("{8C3F1CDD-6173-4591-AEBD-A56A53CA77C1}") + iidDownloadCompleted = ole.NewGUID("{77254866-9F5B-4C8E-B9E2-C77A8530D64B}") + iidInstallProgress = ole.NewGUID("{E01402D5-F8DA-43BA-A012-38894BD048F1}") + iidInstallCompleted = ole.NewGUID("{45F4F6F3-D602-4F98-9A8A-3EFA152AD2D3}") ) func ncQueryInterface(this, iid, ppvObject uintptr) uintptr { guid := (*ole.GUID)(unsafe.Pointer(iid)) out := (*uintptr)(unsafe.Pointer(ppvObject)) - if ole.IsEqualGUID(guid, ole.IID_IUnknown) || ole.IsEqualGUID(guid, ole.IID_IDispatch) { - p := (*noopCallback)(unsafe.Pointer(this)) - p.ref++ + p := (*noopCallback)(unsafe.Pointer(this)) + + // Delegate IMarshal to the free-threaded marshaler so the object is agile and + // WUA can invoke it from its own apartment without a (broken) proxy. + if p.ftm != 0 && ole.IsEqualGUID(guid, iidIMarshal) { + return comQueryInterface(p.ftm, iidIMarshal, out) + } + + if ole.IsEqualGUID(guid, ole.IID_IUnknown) || + ole.IsEqualGUID(guid, iidSearchCompleted) || + ole.IsEqualGUID(guid, iidDownloadProgress) || + ole.IsEqualGUID(guid, iidDownloadCompleted) || + ole.IsEqualGUID(guid, iidInstallProgress) || + ole.IsEqualGUID(guid, iidInstallCompleted) { + atomic.AddInt32(&p.ref, 1) if out != nil { *out = this } return hrSOK } + if out != nil { *out = 0 } @@ -75,35 +120,29 @@ func ncQueryInterface(this, iid, ppvObject uintptr) uintptr { func ncAddRef(this uintptr) uintptr { p := (*noopCallback)(unsafe.Pointer(this)) - p.ref++ - return uintptr(uint32(p.ref)) + return uintptr(uint32(atomic.AddInt32(&p.ref, 1))) } func ncRelease(this uintptr) uintptr { p := (*noopCallback)(unsafe.Pointer(this)) - p.ref-- - return uintptr(uint32(p.ref)) + return uintptr(uint32(atomic.AddInt32(&p.ref, -1))) } -func ncGetTypeInfoCount(this, pctinfo uintptr) uintptr { - if pctinfo != 0 { - *(*uint32)(unsafe.Pointer(pctinfo)) = 0 - } +// ncInvoke is the slot-3 method for every WUA callback interface, e.g. +// ISearchCompletedCallback::Invoke(ISearchJob*, ISearchCompletedCallbackArgs*). +// We ignore the arguments and return S_OK; completion is detected through the +// blocking EndXxx methods and progress through IXxxJob.GetProgress(). +func ncInvoke(this, job, args uintptr) uintptr { return hrSOK } -func ncGetTypeInfo(this, iTInfo, lcid, ppTInfo uintptr) uintptr { - return hrENotImpl -} - -func ncGetIDsOfNames(this, riid, rgszNames, cNames, lcid, rgDispId uintptr) uintptr { - return hrENotImpl -} - -// ncInvoke : no-op body. WUA calls DISPID 0 on progress/completion; we ignore it -// and return S_OK. Completion is detected through EndXxx (blocking). -func ncInvoke(this, dispIdMember, riid, lcid, wFlags, pDispParams, pVarResult, pExcepInfo, puArgErr uintptr) uintptr { - return hrSOK +// comQueryInterface calls IUnknown::QueryInterface (vtable slot 0) on a raw COM +// object pointer, used to fetch IMarshal from the aggregated FTM. +func comQueryInterface(unk uintptr, iid *ole.GUID, out *uintptr) uintptr { + vtbl := *(*uintptr)(unsafe.Pointer(unk)) // first field is the vtable pointer + pQI := *(*uintptr)(unsafe.Pointer(vtbl)) // slot 0 = QueryInterface + ret, _, _ := syscall.SyscallN(pQI, unk, uintptr(unsafe.Pointer(iid)), uintptr(unsafe.Pointer(out))) + return ret } var ( @@ -111,29 +150,47 @@ var ( noopOnce sync.Once keepAliveMu sync.Mutex keepAlive []*noopCallback // pin the objects so the GC does not collect them while WUA holds them + + modole32 = syscall.NewLazyDLL("ole32.dll") + procCoCreateFreeThreadedMarshaler = modole32.NewProc("CoCreateFreeThreadedMarshaler") ) func getNoopVtbl() *noopCallbackVtbl { noopOnce.Do(func() { noopVtbl = &noopCallbackVtbl{ - pQueryInterface: syscall.NewCallback(ncQueryInterface), - pAddRef: syscall.NewCallback(ncAddRef), - pRelease: syscall.NewCallback(ncRelease), - pGetTypeInfoCount: syscall.NewCallback(ncGetTypeInfoCount), - pGetTypeInfo: syscall.NewCallback(ncGetTypeInfo), - pGetIDsOfNames: syscall.NewCallback(ncGetIDsOfNames), - pInvoke: syscall.NewCallback(ncInvoke), + pQueryInterface: syscall.NewCallback(ncQueryInterface), + pAddRef: syscall.NewCallback(ncAddRef), + pRelease: syscall.NewCallback(ncRelease), + pInvoke: syscall.NewCallback(ncInvoke), } }) return noopVtbl } -// newNoopDispatch creates a minimal IDispatch usable as a WUA callback. -// The object is pinned (keepAlive) so it is not collected while WUA holds it. -func newNoopDispatch() *ole.IDispatch { +// newNoopCallback creates a minimal, agile IUnknown usable as a WUA async +// callback. The object is pinned (keepAlive) so it is not collected while WUA +// holds it. COM must already be initialized on the calling thread. +func newNoopCallback() *ole.IUnknown { cb := &noopCallback{lpVtbl: getNoopVtbl(), ref: 1} keepAliveMu.Lock() keepAlive = append(keepAlive, cb) keepAliveMu.Unlock() - return (*ole.IDispatch)(unsafe.Pointer(cb)) + + // Aggregate the free-threaded marshaler so the callback is agile. The + // controlling unknown is the callback itself; the FTM delegates non-IMarshal + // QueryInterface calls back to us. If this fails we leave ftm=0 and fall back + // to standard marshaling (BeginXxx may then fail and the caller falls back to + // the synchronous path). + var ftm uintptr + if procCoCreateFreeThreadedMarshaler.Find() == nil { + ret, _, _ := procCoCreateFreeThreadedMarshaler.Call( + uintptr(unsafe.Pointer(cb)), + uintptr(unsafe.Pointer(&ftm)), + ) + if ret == 0 { + cb.ftm = ftm + } + } + + return (*ole.IUnknown)(unsafe.Pointer(cb)) } diff --git a/olecallback_test.go b/olecallback_test.go new file mode 100644 index 0000000..95d0ee5 --- /dev/null +++ b/olecallback_test.go @@ -0,0 +1,75 @@ +//go:build windows +// +build windows + +/* +Copyright 2022 Zheng Dayu +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package windowsupdate + +import ( + "testing" + "unsafe" + + "github.com/go-ole/go-ole" +) + +func TestNewNoopCallback_SharedSingleton(t *testing.T) { + first := newNoopCallback() + if first == nil { + t.Fatal("newNoopCallback returned nil") + } + second := newNoopCallback() + if first != second { + t.Errorf("expected newNoopCallback to return the shared singleton, got %p and %p", first, second) + } +} + +func TestNoopCallback_QueryInterface(t *testing.T) { + cb := getNoopVtbl() + this := uintptr(unsafe.Pointer(cb)) + + // A requested IID of IUnknown or IDispatch must succeed and return the object. + for _, iid := range []*ole.GUID{ole.IID_IUnknown, ole.IID_IDispatch} { + var out uintptr + hr := ncQueryInterface(this, uintptr(unsafe.Pointer(iid)), uintptr(unsafe.Pointer(&out))) + if hr != hrSOK { + t.Errorf("QueryInterface(%v) = 0x%x, want S_OK", iid, hr) + } + if out != this { + t.Errorf("QueryInterface(%v) out = 0x%x, want 0x%x", iid, out, this) + } + } + + // An unsupported IID must fail with E_NOINTERFACE and clear the out pointer. + unsupported := &ole.GUID{Data1: 0xdeadbeef, Data2: 0x1234, Data3: 0x5678} + out := uintptr(0xfff) + hr := ncQueryInterface(this, uintptr(unsafe.Pointer(unsupported)), uintptr(unsafe.Pointer(&out))) + if hr != hrENoInterface { + t.Errorf("QueryInterface(unsupported) = 0x%x, want E_NOINTERFACE", hr) + } + if out != 0 { + t.Errorf("QueryInterface(unsupported) out = 0x%x, want 0", out) + } +} + +func TestNoopCallback_AddRefRelease(t *testing.T) { + cb := &noopCallback{lpVtbl: getNoopVtbl(), ref: 1} + this := uintptr(unsafe.Pointer(cb)) + + if got := ncAddRef(this); got != 2 { + t.Errorf("AddRef = %d, want 2", got) + } + if got := ncRelease(this); got != 1 { + t.Errorf("Release = %d, want 1", got) + } +} From 4ea1b8a0b5a7c5979b9217c1dc916c2e7a900001 Mon Sep 17 00:00:00 2001 From: Sabristi <65233663+wisper-sabrina@users.noreply.github.com> Date: Tue, 23 Jun 2026 09:17:39 +0200 Subject: [PATCH 3/5] Update olecallback_test.go Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- olecallback_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/olecallback_test.go b/olecallback_test.go index 95d0ee5..d1f3700 100644 --- a/olecallback_test.go +++ b/olecallback_test.go @@ -38,8 +38,8 @@ func TestNoopCallback_QueryInterface(t *testing.T) { cb := getNoopVtbl() this := uintptr(unsafe.Pointer(cb)) - // A requested IID of IUnknown or IDispatch must succeed and return the object. - for _, iid := range []*ole.GUID{ole.IID_IUnknown, ole.IID_IDispatch} { + // A requested IID of IUnknown or a supported WUA callback IID must succeed and return the object. + for _, iid := range []*ole.GUID{ole.IID_IUnknown, iidSearchCompleted} { var out uintptr hr := ncQueryInterface(this, uintptr(unsafe.Pointer(iid)), uintptr(unsafe.Pointer(&out))) if hr != hrSOK { From a655f70292653ddd694fe0e91c9292ff74fa6c1b Mon Sep 17 00:00:00 2001 From: Zheng Dayu Date: Mon, 29 Jun 2026 16:41:18 +0800 Subject: [PATCH 4/5] refactor: use singleton noopCallback to fix memory leak - Replace per-call allocation + keepAlive slice with sync.Once singleton - Fix olecallback_other.go stub to match new function signature - Fix TestNoopCallback_QueryInterface to use proper noopCallback object - Clean up stale IDispatch comments in olecallback.go Change-Id: I660e3afeb48e38ec4ad225e69c48365b83192da1 Co-developed-by: Qoder --- olecallback.go | 69 ++++++++++++++++++++++---------------------- olecallback_other.go | 4 +-- olecallback_test.go | 2 +- 3 files changed, 38 insertions(+), 37 deletions(-) diff --git a/olecallback.go b/olecallback.go index 477dcba..a2e1c07 100644 --- a/olecallback.go +++ b/olecallback.go @@ -51,11 +51,6 @@ import ( // Free-Threaded Marshaler (FTM): QueryInterface(IID_IMarshal) is delegated to the // FTM, which makes the object agile (callable directly from any apartment, no // proxy). BeginXxx then succeeds and progress is read by polling IXxxJob.GetProgress(). -// a non-NULL IUnknown* callback argument. Passing NULL (VT_NULL) makes them fail -// with DISP_E_TYPEMISMATCH (0x80020005). newNoopDispatch returns a minimal -// IDispatch whose Invoke does nothing (returns S_OK): completion is obtained -// through the blocking EndXxx methods and progress through IXxxJob.GetProgress(). -// // The handler signatures are 100% uintptr because that is required by // syscall.NewCallback. @@ -151,10 +146,10 @@ func comQueryInterface(unk uintptr, iid *ole.GUID, out *uintptr) uintptr { } var ( - noopVtbl *noopCallbackVtbl - noopOnce sync.Once - keepAliveMu sync.Mutex - keepAlive []*noopCallback // pin the objects so the GC does not collect them while WUA holds them + noopVtbl *noopCallbackVtbl + noopOnce sync.Once + globalNoopCb *noopCallback + globalNoopMu sync.Once modole32 = syscall.NewLazyDLL("ole32.dll") procCoCreateFreeThreadedMarshaler = modole32.NewProc("CoCreateFreeThreadedMarshaler") @@ -172,30 +167,36 @@ func getNoopVtbl() *noopCallbackVtbl { return noopVtbl } -// newNoopCallback creates a minimal, agile IUnknown usable as a WUA async -// callback. The object is pinned (keepAlive) so it is not collected while WUA -// holds it. COM must already be initialized on the calling thread. -func newNoopCallback() *ole.IUnknown { - cb := &noopCallback{lpVtbl: getNoopVtbl(), ref: 1} - keepAliveMu.Lock() - keepAlive = append(keepAlive, cb) - keepAliveMu.Unlock() - - // Aggregate the free-threaded marshaler so the callback is agile. The - // controlling unknown is the callback itself; the FTM delegates non-IMarshal - // QueryInterface calls back to us. If this fails we leave ftm=0 and fall back - // to standard marshaling (BeginXxx may then fail and the caller falls back to - // the synchronous path). - var ftm uintptr - if procCoCreateFreeThreadedMarshaler.Find() == nil { - ret, _, _ := procCoCreateFreeThreadedMarshaler.Call( - uintptr(unsafe.Pointer(cb)), - uintptr(unsafe.Pointer(&ftm)), - ) - if ret == 0 { - cb.ftm = ftm +// newNoopCallback returns a shared singleton usable as a WUA async callback. +// Because the callback is completely stateless (ncInvoke is a no-op), a single +// instance can be safely shared across all async calls. This avoids the +// unbounded memory growth that would result from allocating a new callback on +// every invocation. COM must already be initialized on the calling thread. +// +// The return type is *ole.IDispatch (not *ole.IUnknown) because go-ole's +// oleutil.CallMethod only handles *IDispatch in its type switch; passing +// *IUnknown causes a panic("unknown type"). The cast is safe: go-ole just +// uses the pointer value to build a VT_DISPATCH VARIANT, and the WUA method +// will QueryInterface our object for the actual callback interface it needs. +func newNoopCallback() *ole.IDispatch { + globalNoopMu.Do(func() { + globalNoopCb = &noopCallback{lpVtbl: getNoopVtbl(), ref: 1} + + // Aggregate the free-threaded marshaler so the callback is agile. The + // controlling unknown is the callback itself; the FTM delegates non-IMarshal + // QueryInterface calls back to us. If this fails we leave ftm=0 and fall back + // to standard marshaling (BeginXxx may then fail and the caller falls back to + // the synchronous path). + var ftm uintptr + if procCoCreateFreeThreadedMarshaler.Find() == nil { + ret, _, _ := procCoCreateFreeThreadedMarshaler.Call( + uintptr(unsafe.Pointer(globalNoopCb)), + uintptr(unsafe.Pointer(&ftm)), + ) + if ret == 0 { + globalNoopCb.ftm = ftm + } } - } - - return (*ole.IUnknown)(unsafe.Pointer(cb)) + }) + return (*ole.IDispatch)(unsafe.Pointer(globalNoopCb)) } diff --git a/olecallback_other.go b/olecallback_other.go index 1a30a3a..5e15d2a 100644 --- a/olecallback_other.go +++ b/olecallback_other.go @@ -17,8 +17,8 @@ package windowsupdate import "github.com/go-ole/go-ole" -// newNoopDispatch is a no-op stub on non-Windows platforms. +// newNoopCallback is a no-op stub on non-Windows platforms. // The COM async methods are only functional on Windows. -func newNoopDispatch() *ole.IDispatch { +func newNoopCallback() *ole.IDispatch { return nil } diff --git a/olecallback_test.go b/olecallback_test.go index d1f3700..476900a 100644 --- a/olecallback_test.go +++ b/olecallback_test.go @@ -35,7 +35,7 @@ func TestNewNoopCallback_SharedSingleton(t *testing.T) { } func TestNoopCallback_QueryInterface(t *testing.T) { - cb := getNoopVtbl() + cb := &noopCallback{lpVtbl: getNoopVtbl(), ref: 1} this := uintptr(unsafe.Pointer(cb)) // A requested IID of IUnknown or a supported WUA callback IID must succeed and return the object. From 1618c187dd1c21b7afb7bd05a0d5191b50d5d48a Mon Sep 17 00:00:00 2001 From: Zheng Dayu Date: Mon, 29 Jun 2026 17:00:49 +0800 Subject: [PATCH 5/5] fix: restore nil safety checks in ncQueryInterface - Return E_POINTER when ppvObject == 0 - Return E_NOINTERFACE when iid == 0 (prevents nil deref on GUID cast) - Add corresponding test cases for both edge cases - Move guid/out dereference after null checks for safety Change-Id: I616164500ed2cf2c1761620bb2fb9ed4fd6a7070 Co-developed-by: Qoder --- olecallback.go | 18 +++++++++++------- olecallback_test.go | 14 ++++++++++++++ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/olecallback.go b/olecallback.go index a2e1c07..dd9f64f 100644 --- a/olecallback.go +++ b/olecallback.go @@ -72,6 +72,7 @@ type noopCallback struct { // HRESULT values as uintptr (only the low 32 bits are significant). const ( hrSOK = uintptr(0x00000000) + hrEPointer = uintptr(0x80004003) hrENoInterface = uintptr(0x80004002) ) @@ -89,8 +90,15 @@ var ( ) func ncQueryInterface(this, iid, ppvObject uintptr) uintptr { - guid := (*ole.GUID)(unsafe.Pointer(iid)) + if ppvObject == 0 { + return hrEPointer + } out := (*uintptr)(unsafe.Pointer(ppvObject)) + if iid == 0 { + *out = 0 + return hrENoInterface + } + guid := (*ole.GUID)(unsafe.Pointer(iid)) p := (*noopCallback)(unsafe.Pointer(this)) // Delegate IMarshal to the free-threaded marshaler so the object is agile and @@ -106,15 +114,11 @@ func ncQueryInterface(this, iid, ppvObject uintptr) uintptr { ole.IsEqualGUID(guid, iidInstallProgress) || ole.IsEqualGUID(guid, iidInstallCompleted) { atomic.AddInt32(&p.ref, 1) - if out != nil { - *out = this - } + *out = this return hrSOK } - if out != nil { - *out = 0 - } + *out = 0 return hrENoInterface } diff --git a/olecallback_test.go b/olecallback_test.go index 476900a..7cdd61d 100644 --- a/olecallback_test.go +++ b/olecallback_test.go @@ -60,6 +60,20 @@ func TestNoopCallback_QueryInterface(t *testing.T) { if out != 0 { t.Errorf("QueryInterface(unsupported) out = 0x%x, want 0", out) } + + // Safety: ppvObject == 0 must return E_POINTER. + if hr := ncQueryInterface(this, uintptr(unsafe.Pointer(ole.IID_IUnknown)), 0); hr != hrEPointer { + t.Errorf("QueryInterface with nil ppvObject = 0x%x, want E_POINTER (0x80004003)", hr) + } + + // Safety: iid == 0 must return E_NOINTERFACE and clear out. + out = uintptr(0xfff) + if hr := ncQueryInterface(this, 0, uintptr(unsafe.Pointer(&out))); hr != hrENoInterface { + t.Errorf("QueryInterface with nil iid = 0x%x, want E_NOINTERFACE", hr) + } + if out != 0 { + t.Errorf("QueryInterface with nil iid out = 0x%x, want 0", out) + } } func TestNoopCallback_AddRefRelease(t *testing.T) {