Terminate COM surrogate process before update/gc#313791
Conversation
Co-authored-by: Copilot <copilot@github.com>
There was a problem hiding this comment.
Pull request overview
Addresses Windows update runs that never complete by proactively terminating the File Explorer context-menu COM surrogate (dllhost.exe /Processid:<CLSID>) that can hold file handles and block inno_updater --gc cleanup (issue #294546).
Changes:
- Add a Win32 update-service step to stop the context-menu COM surrogate before invoking
inno_updater --gc. - Add an Inno Setup helper to stop the same COM surrogate before AppX removal and before
--gccleanup. - Introduce
win32ContextMenuproduct configuration and wire its CLSID into the Win32 setup build definitions.
Show a summary per file
| File | Description |
|---|---|
| src/vs/platform/update/electron-main/updateService.win32.ts | Runs a PowerShell-based kill step for the context-menu COM surrogate before background-update GC. |
| src/vs/base/common/product.ts | Adds win32ContextMenu to product configuration typing. |
| build/win32/code.iss | Adds KillContextMenuComSurrogate() and invokes it prior to AppX removal and GC. |
| build/gulpfile.vscode.win32.ts | Passes FileExplorerContextMenuCLSID define to Inno Setup when available from product metadata. |
Copilot's findings
Comments suppressed due to low confidence (1)
src/vs/platform/update/electron-main/updateService.win32.ts:206
- The exit handler logs at info level regardless of whether PowerShell succeeded. If
codeis non-zero, this should be treated as a failure (warn/error) so we don’t silently miss the cleanup step; conversely a0exit could be logged at trace to reduce log noise.
}).once('exit', code => {
this.logService.info(`update#killContextMenuComSurrogate: powershell exited with code ${code}`);
resolve();
- Files reviewed: 4/4 changed files
- Comments generated: 2
|
Pipeline with latest builds: https://dev.azure.com/monacotools/Monaco/_build/results?buildId=435734&view=results |
Screenshot ChangesBase: Changed (26) |
deepak1556
left a comment
There was a problem hiding this comment.
Could we just run this at RemoveAppxPackage which is more definitive point in time where we want the old appx package to be removed, this step always runs during update flow. I don't see why we also need to call it before gc.
For background update for a user setup we run --gc from from UpdateService - by that time the file can be locked again. There are other scenarios and even when we do and there is a wait, the file can be locked again - so we would be blocked. |
Why would that be, we have unregistered the appx corresponding to the old version if anything is running at the time of gc it should be from the new version which gc will not even touch in the first place. Could you share steps that demonstrate the issue of old dll holding up process after removing the appx package ? |
Fixes #294546
Fixes #295844
Fixes #312098
Fixes #290467
Fixes #313585
The fix for the mutex race is necessary since under locking conditions uninstall of AppX package may take some time and we are restarting the app by that time, but setup is still running.
I was able to repro this on 1.118.1 with FreeCommander XE 32-bit.
Verified the fix with both user and system setup/update.
Co-authored-by: Copilot copilot@github.com