Skip to content

ignore process permission denied - #28

Open
Peter Pickford (ppickfor) wants to merge 1 commit into
microsoft:mainfrom
ppickfor:ignore-insufficient-permissions
Open

ignore process permission denied#28
Peter Pickford (ppickfor) wants to merge 1 commit into
microsoft:mainfrom
ppickfor:ignore-insufficient-permissions

Conversation

@ppickfor

Copy link
Copy Markdown

microsoft/vscode#76641

This ignores the error from OpenProcess() if it is access denied.

Provided the error value and GetLastError() call are correct and imported correctly.

If you can get the process information you can't terminate the process.

Hopefully this fixes the you can't upgrade a user installed vs code if other users are running vs code on a multi user system.

@ppickfor

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

@ppickfor

Copy link
Copy Markdown
Author

@joaomoreno

Copy link
Copy Markdown
Contributor

Hopefully this fixes the you can't upgrade a user installed vs code if other users are running vs code on a multi user system.

I don't think this changes anything. If we fail to kill the process, it will just stay running, which means the update functionality won't be able to overwrite the binary that is running. What am I missing here?

@joaomoreno João Moreno (joaomoreno) added the info-needed Issue requires more information from poster label Oct 28, 2024
@ppickfor

Copy link
Copy Markdown
Author

@MrFly72

Copy link
Copy Markdown

Just out of the blue, as I am not a C/C# programmer and tested it with Powershell:
I like the idea to check if the process information can be gathered. So for me its these situations:

  1. Running Unelevated: All processes, we cannot get informations on should be from someone else and should be ignored
  2. Running Elevated: As I get the process informations, we can check on the Path of the process and "see" if the installer would overwrite exactly THAT .exe or not and can therefor exclude all processes, that did not get run from that path, where we want to install into?
    Does that make sense?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

info-needed Issue requires more information from poster

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants