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 9a6efa4..dd9f64f 100644 --- a/olecallback.go +++ b/olecallback.go @@ -1,6 +1,7 @@ //go:build windows /* +Copyright 2022 Zheng Dayu Copyright 2026 Zheng Dayu Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -25,31 +26,47 @@ 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. -// noopCallbackVtbl is the COM virtual function table layout for IDispatch. -// The order of fields MUST match the IUnknown + IDispatch v-table layout. 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 is a stateless dummy IDispatch implementation. lpVtbl MUST be -// the first field because the COM interface pointer points directly to it. +// noopCallback : lpVtbl MUST be the first field (the COM interface pointer points +// to it). 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). @@ -57,7 +74,19 @@ const ( hrSOK = uintptr(0x00000000) hrEPointer = uintptr(0x80004003) 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 { @@ -70,73 +99,108 @@ func ncQueryInterface(this, iid, ppvObject uintptr) uintptr { return hrENoInterface } guid := (*ole.GUID)(unsafe.Pointer(iid)) - if ole.IsEqualGUID(guid, ole.IID_IUnknown) || ole.IsEqualGUID(guid, ole.IID_IDispatch) { - atomic.AddInt32(&globalNoop.ref, 1) + 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) *out = this return hrSOK } + *out = 0 return hrENoInterface } func ncAddRef(this uintptr) uintptr { - return uintptr(uint32(atomic.AddInt32(&globalNoop.ref, 1))) + p := (*noopCallback)(unsafe.Pointer(this)) + return uintptr(uint32(atomic.AddInt32(&p.ref, 1))) } func ncRelease(this uintptr) uintptr { - // Singleton object: it is never actually freed even if the count reaches - // zero. We still maintain the counter so the value returned to the COM - // caller is meaningful. - return uintptr(uint32(atomic.AddInt32(&globalNoop.ref, -1))) + p := (*noopCallback)(unsafe.Pointer(this)) + 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 { - if pVarResult != 0 { - v := (*ole.VARIANT)(unsafe.Pointer(pVarResult)) - v.VT = ole.VT_EMPTY - } - 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 ( - noopOnce sync.Once - globalNoop *noopCallback + noopVtbl *noopCallbackVtbl + noopOnce sync.Once + globalNoopCb *noopCallback + globalNoopMu sync.Once + + modole32 = syscall.NewLazyDLL("ole32.dll") + procCoCreateFreeThreadedMarshaler = modole32.NewProc("CoCreateFreeThreadedMarshaler") ) -// newNoopDispatch returns a pointer to a global singleton IDispatch usable as -// a WUA callback. Because the callback is completely stateless, a single -// instance can be safely shared across all async calls. This avoids the -// unbounded memory leak that would result from allocating a new callback on -// every invocation and pinning it in a global slice. -func newNoopDispatch() *ole.IDispatch { +func getNoopVtbl() *noopCallbackVtbl { noopOnce.Do(func() { - vtbl := &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), + noopVtbl = &noopCallbackVtbl{ + pQueryInterface: syscall.NewCallback(ncQueryInterface), + pAddRef: syscall.NewCallback(ncAddRef), + pRelease: syscall.NewCallback(ncRelease), + pInvoke: syscall.NewCallback(ncInvoke), + } + }) + return noopVtbl +} + +// 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 + } } - globalNoop = &noopCallback{lpVtbl: vtbl, ref: 1} }) - return (*ole.IDispatch)(unsafe.Pointer(globalNoop)) + 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 new file mode 100644 index 0000000..7cdd61d --- /dev/null +++ b/olecallback_test.go @@ -0,0 +1,89 @@ +//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 := &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. + 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 { + 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) + } + + // 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) { + 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) + } +}