[Bug]: app.requestSingleInstanceLock() returns true on second start (Windows)

[ericpromislow]

Preflight Checklist

• I have read the Contributing Guidelines for this project.
• I agree to follow the Code of Conduct that this project adheres to.
• I have searched the issue tracker for a bug report that matches the one I want to file, without success.

Electron Version

18.0.3

What operating system are you using?

Windows

Operating System Version

Windows 10 Pro 19043.1889

What arch are you using?

x64

Last Known Working Electron version

n/a

Expected Behavior

I've "compiled" our app into a standalone Windows executable. When I click on the app's icon the first time,
I expect the app to run. When I click on the icon the second time, I want to see the first instance's window,
even if it was minimized.

I do this through the following code:

const lockInfo = { otherPid: -1 };
if (!Electron.app.requestSingleInstanceLock(lockInfo)) {
  gone = true;
  Electron.app.quit();
}
...

Electron.app.on('second-instance', async(_event, _commandLine, _workingDirectory, additionalData: any) => {
  // Someone tried to run another instance,
  // reveal and focus this window instead.
  if (additionalData) {
    additionalData.otherPid = process.pid;
  } else {
    console.log(`additionalData not an object`);
  }
  await protocolRegistered;
  window.openMain();
});

Actual Behavior

Here's the gist of the code, running in the main process's main TS file:

const lockInfo = { otherPid: -1 };
if (!Electron.app.requestSingleInstanceLock(lockInfo) || lockInfo.otherPid !== -1) {
  gone = true;
  Electron.app.quit();
}

// So if we got the single-instance lock but the lockInfo.otherPid is no longer -1, it means another process 
// is telling us they were first and we should shut down.
 
//…

Electron.app.on('second-instance', async(_event, _commandLine, _workingDirectory, additionalData: any) => {
  // Someone tried to run another instance of Rancher Desktop,
  // reveal and focus this window instead.
  if (additionalData) {
    additionalData.otherPid = process.pid;
  }
  await protocolRegistered;
  window.openMain();
});

Both instances succeed at calling Electron.app.requestSingleInstanceLock()

The first instance gets a second-instance event, but the additionalInfo field is empty. When I saw that
the call to requestSingleInstanceLock() was always succeeding, I was hoping that if I passed an object to requestSingleInstanceLock() it would be passed as additionalInfo to the second-instance event handler, but that isn't happening.

Testcase Gist URL

Nope

Additional Information

Here's how to repro:

cd ... some-working-directory
git clone https://github.com/rancher-sandbox/rancher-desktop.git
npm run build
./dist/Rancher<TAB> complete

Running the installer takes a few minutes to show up.
Then double-click on the icon, and allow it to start up. Minimize the window.
Then double-click on the icon again. It should bring up the window for the first instance, but instead
it creates a new window and emits a message that a port is already in use.

There's a lockfile in ~/AppData/Roaming/rancher-desktop, but it doesn't seem to be working the same way the Singleton* files in ~/L/AppSupp/r-d do for macOS.

[rzhao271]

if (additionalData) {
    additionalData.otherPid = process.pid;
}

additionalData is passed from the second instance to the first instance. Changing it in the event handler doesn't affect the second instance because the variable in that case is only in the first instance.

Also see #34279, though it hasn't been maintained.

[ericpromislow]

I read the code about adding message passing to the single-instance-lock handler.

Note that the issue here is that the second process was able to get the single-instance-lock -- our code never releases it. That's my core problem. The call works as expected on macOS (meaning that the second instance fails to get the lock), but doesn't on Windows (the second instance succeeds at getting the lock when it I would expect it not to).

The use of additionalData. otherPid was my attempt to find a workaround. I'm more interested in either finding out how to help fix the problem on Windows, or find out about a better workaround.

[VerteDinde]

@ericpromislow I noticed you're using v18.0.3 - could you please try using 18.3.0 or higher? We fixed fairly significant issues with requestSingleInstanceLock in both 18.2.0 and 18.3.0, bumping up to 18.3.0 may resolve this issue for you: https://www.electronjs.org/releases/stable?version=18&page=2#18.3.0

[ericpromislow]

Bumping to 18.3.6 didn't fix this issue on Windows.

[adamkpickering]

Are there any updates on this? While I haven't gone into it as far as @ericpromislow, I'm seeing requestSingleInstanceLock() fail in the same way on Linux.

What is the likelihood that someone in the know will be able to look at this? I've read some of the electron and chromium code around this, and I'm not sure I could implement a fix without a lot of effort, given I'm not familiar with C++...

[rzhao271]

I wonder how much of it is due to the second-instance handler being async?
Was there a previous Electron version where the API worked for y'all?

[adamkpickering]

I wonder how much of it is due to the second-instance handler being async?

I did some investigation on Linux with strace at one point. I was able to verify that the communication between the first instance and the second instance was working as expected, i.e. the second instance sent some data to the first instance, and then the first instance replied with ACK and closed its end of the connection. So I don't think the problem is with the second-instance handler being async. I think there is some issue with the code that deals with what happens after ACK is received from the first instance. But unfortunately I didn't get any further.

Was there a previous Electron version where the API worked for y'all?

I don't think so... @ericpromislow do you know?

[ericpromislow]

I'm pretty sure it did work, but don't remember which version it broke on (I could always investigate...)

[adamkpickering]

So I had time to do some more digging and I figured out that the problem was on our side. Basically we were calling app.requestSingleInstanceLock twice: once before changing the application name with app.setName, and once after. The application name determines where the files used to check whether the application is a second instance are put (in our case on Linux, ~/.config/Rancher Desktop/SingletonLock etc). So, the second instance was getting confused by this change in location.

So, this issue can be closed. However, I feel like this wouldn't have happened if electron's design around application name was slightly different. My understanding is that there are two versions of the application name: the "pretty" version ("Rancher Desktop" in our case) and the "programmatic" version ("rancher-desktop"). We need to set the pretty version because that is what appears in the title bar. But it seems strange that the pretty name is the one that is used to determine the directory where all of the electron/chromium stuff is cached (i.e. ~/.config/Rancher Desktop). Why not use the programmatic name?

More feedback: I had a bear of a time trying to move the location of the electron/chromium cache directory. I wasn't actually successful in the end; I left it at its default value of ~/.config/Rancher Desktop. This is not ideal, since we now have two directories under ~/.config that belong to Rancher Desktop: ~/.config/Rancher Desktop and ~/.config/rancher-desktop. It seems that electron creates ~/.config/Rancher Desktop on startup and there is no way to stop that from happening.

Also, why is the default location for cache files under ~/.config? A location like ~/.local/state, ~/.local/share or ~/.cache would be more appropriate according to the XDG spec.

Sorry for all the criticisms. Maybe you guys are aware of them, and maybe they've been fixed since the version of electron we are using. But I figured it wouldn't hurt to note them here.

[github-actions]

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. If you have any new additional information—in particular, if this is still reproducible in the latest version of Electron or in the beta—please include it with your comment!

[github-actions]

This issue has been closed due to inactivity, and will not be monitored. If this is a bug and you can reproduce this issue on a supported version of Electron please open a new issue and include instructions for reproducing the issue.