fix: Rename the old server before update

[lnicola]

Kinda' fixes #6602

[lnicola]

r? @wxb1ank

[lnicola]

Not sure what entry is here.

[wxb1ank]

Sorry if I'm missing something, but how is temporarily renaming the server to 'stale' different than replacing it with the newly downloaded one? I haven't tested, but if one or more server instances are running, won't this always cause EPERM?

[lnicola]

It seems that you actually can rename a running executable on Windows. But yeah, this is untested otherwise.

[wxb1ank]

I feel like we wouldn't have run into the original permission error if that were the case. IMO, the simplest thing to do here is to check specifically for EPERM and log something ala "Failed to overwrite the old server executable. Are there instances currently running?". Needing to update RA while it's running seems like a niche case to me.

[lnicola]

Yeah, I'd agree, but this seems to bother a lot of people. So while it's not a full solution, at least it might take us 90% of the way there.

[wxb1ank]

Well, the code LGTM otherwise. The first Google result I found on this topic implied this couldn't be done, but this suggests otherwise? I'll try and test this later today or tomorrow.

[wxb1ank]

I tested with two VS Code windows open. One was running RA in the background while the other attempted to update.

DEBUG [7/12/2021, 5:57:01 PM]: Downloading file of 254131 bytes size from https://github.com/rust-analyzer/rust-analyzer/releases/download/nightly/rust-analyzer.vsix to c:\Users\wxblank\AppData\Roaming\Code\User\globalStorage\matklad.rust-analyzer\rust-analyzer169d3161c8
ERROR [7/12/2021, 5:57:02 PM]: Cannot rename running instance: EntryNotFound (FileSystemError) (FileSystemError): Error: ENOENT: no such file or directory, rename 'c:\Users\wxblank\AppData\Roaming\Code\User\globalStorage\matklad.rust-analyzer\rust-analyzer.vsix' -> 'c:\Users\wxblank\AppData\Roaming\Code\User\globalStorage\matklad.rust-analyzer\rust-analyzer-stale-169d3161c8.vsix'
ERROR [7/12/2021, 5:57:02 PM]: Bootstrap error [Error: ENOENT: no such file or directory, open 'c:\Users\wxblank\AppData\Roaming\Code\User\globalStorage\matklad.rust-analyzer\rust-analyzer.vsix',    at h (file:///C:/Program Files/Microsoft VS Code/resources/app/out/vs/code/electron-browser/sharedProcess/sharedProcessMain.js:54:176162),    at file:///C:/Program Files/Microsoft VS Code/resources/app/out/vs/code/electron-browser/sharedProcess/sharedProcessMain.js:54:177561,    at C:\Program Files\Microsoft VS Code\resources\app\node_modules.asar\yauzl\index.js:34:21,    at FSReqCallback.oncomplete (fs.js:171:23)]

https://github.com/rust-analyzer/rust-analyzer/blob/5280b6d84c4fc727d055b02239d631963264cb2b/editors/code/src/net.ts#L127
It seems this doesn't work. VS Code docs strike again! EntryNotFound was thrown instead of the expected FileNotFound. We could just hardcode the first one instead, but I think a more resilient solution is as follows:

1. Catch and ignore this particular error.
2. Catch and handle the error from replacing the old binary with the new, downloaded one. Currently, we don't catch this one at all, and I think it's the reason I logged a bootstrap error. However, I'm a little confused why I get an error at all.

[wxb1ank]

With the following:

// Try to rename a running server to avoid EPERM on Windows
// NB: this can lead to issues if a running Code instance tries to restart the server.
try {
    await vscode.workspace.fs.rename(opts.dest, oldServerPath, { overwrite: true });
    log.info(`Renamed old server binary ${opts.dest.fsPath} to ${oldServerPath.fsPath}`);
} catch (err) {

}

try {
    await vscode.workspace.fs.rename(tempFilePath, opts.dest, { overwrite: true });
} catch (err) {
    log.error(`Failed to replace the old binary with the new one: ${err}`);
}

I no longer receive the previous error, and the update seems to go smoothly (with the old RA instance still active).

[lnicola]

I ended up checking for both, to avoid showing an error when there's no existing binary, but still complain when something happens. Of course vscode.workspace.fs.stat also throws on missing files.

Also, thanks a lot for giving this a try!

[lnicola]

I think I want to merge this today so we can test it a little before next week (unless someone r-'es it, of course).

[matklad]

bors r+

[bors]

Build succeeded:

• Rust (macos-latest)
• Rust (ubuntu-latest)
• Rust (windows-latest)
• TypeScript (ubuntu-latest)
• TypeScript (windows-latest)