fix: create singleton pipename from user & executable

[VerteDinde]

Description of Change

While investigating #33975, we realized that the problem is not limited to just running the same version of one app in two user sessions, but any two Electron apps using 18 or higher in two user sessions. For example: If a user runs an app using Electron 18+ in one Windows user session, and then run a simple Fiddle that calls app.requestSingleInstanceLock() in another Windows user session, the Fiddle crashes.

The problem is that named pipes are not session-specific, and thus all Electron apps are trying to access the same named pipe.

This PR adjusts the pipe name, so that it is more unique per executable and per user.

Checklist

• PR description included and stakeholders cc'd
• npm test passes
• tests are changed or added
• relevant documentation is changed or added
• PR release notes describe the change in a way relevant to app developers, and are capitalized, punctuated, and past tense.

Release Notes

Notes: Fixed a crash on Windows when opening apps in multiple, separate user sessions.

[deepak1556]

I like the temporary solution, maybe we can make this bit more unique. Since it is the first instance that currently creates the named pipe which also turns out to be the remote_window which other instances talk to, what if we create the pipe name with program_name, first instance process id, first instance main thread id and a random integer that is written to the kLockfile. The lock file value would allow us to connect the pipe of instances operating under the same user data dir.

// First instance end
uint64_t rand = base::RandUint64();
base::WriteFile(lock_file_path, rand);
std::wstring pipe_name = base::StringPrintf(
      L"\\\\.\\pipe\\%s.%lu.%lu.%I64u", program_name_, ::GetCurrentProcessId(),
      ::GetCurrentThreadId(), rand);

// Second instance end
int64_t file_size = 0;
base::GetFileSize(lock_file_path, &file_size);
std::string contents(file_size, '\0');
base::ReadFile(lock_file_path, &contents[0], file_size);
DWORD process_id;
DWORD thread_id = GetWindowThreadProcessId(remote_window_, &process_id);
std::wstring pipe_name = base::StringPrintf(
      L"\\\\.\\pipe\\%s.%lu.%lu.%s", program_name_, process_id,
      thread_id, contents);

[VerteDinde]

I’m fine with that, will make the change! One complication with this approach is that pipe names are limited to 256 chars, so I may need to do some abbreviation on items like the program name to make sure we don’t risk exceeding the limit in certain cases - I’ll test and make sure 👍

[deepak1556]

so I may need to do some abbreviation on items like the program name to make sure we don’t risk exceeding the limit in certain cases

program_name_ can be omitted in that case, the other parameters should provide the required uniqueness.

[VerteDinde]

@deepak1556 @rzhao271 I tried a few different techniques to get the rand number between processes, including writing to the lockfile and creating a new temporary file. It seems that the lockfile (and any other file created/edited) is marked as used by the current process, and permissions aren't granted to edit.

To get around this, I reintroduced adding the user_name to the pipename, but also kept the program_name, first instance process id and first instance main thread id. Between these four values, I think we should have a fairly high chance of uniqueness for this fix - tests are passing both locally and on CI for me with this approach.

Let me know if you have any concerns with this approach, happy to try something else!

[deepak1556]

Left minor style comments.

Do we need user name ? Process ID and Thread ID are unique to the system across sessions, so I feel they should suffice for now ? Also, user name might make the pipe name length exceed its limit.

[VerteDinde]

Removed user_name after review; using it was also failing consistently on ia32.

[release-clerk]

Release Notes Persisted

Fixed a crash on Windows when opening apps in multiple, separate user sessions.

[trop]

I was unable to backport this PR to "18-x-y" cleanly;
you will need to perform this backport manually.

[trop]

I have automatically backported this PR to "19-x-y", please check out #34160

[trop]

@VerteDinde has manually backported this PR to "18-x-y", please check out #34161