Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Theia launched from Theia hangs on Windows [Electron] #11081 #11082

Merged
merged 1 commit into from
Apr 28, 2022

Conversation

jfaltermeier
Copy link
Contributor

@jfaltermeier jfaltermeier commented Apr 27, 2022

What it does

  • Check existing environment for variants of the ElectronSecurityToken key on windows

fixes #11081

How to test

  • Launch the Theia Electron Example on windows
  • Inside the running example, open a workspace with a second clone of the Theia sources, build examples, and launch the Theia Electron Example
  • The second electron application should start

Review checklist

Reminder for reviewers

Copy link
Member

@paul-marechal paul-marechal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that setting the key to uppercase fixes the issue on Windows, according to your change.

Ultimately it would be nice if we could understand why using uppercase works and lowercase doesn't, but let's just use uppercase.

export const ElectronSecurityToken = 'x-theia-electron-token';

To:

/**
 * This token is unique to the current running instance. It is used by the backend
 * to make sure it is an electron browser window that is connecting to its services.
 *
 * The identifier is a string, which makes it usable as a key for cookies, environments, etc.
 */
// Note that it needs to be uppercase for it to work properly in Windows environments.
export const ElectronSecurityToken: string = 'THEIA_ELECTRON_TOKEN';

(I put the note as an inline comment to not pollute the public jsdoc with details)

@jfaltermeier
Copy link
Contributor Author

Thanks, I will change it to upper case and update the PR. The information I found was this: https://nodejs.org/api/child_process.html


On Windows, environment variables are case-insensitive. Node.js lexicographically sorts the env keys and uses the first one that case-insensitively matches. Only first (in lexicographic order) entry will be passed to the subprocess. This might lead to issues on Windows when passing objects to the env option that have multiple variants of the same key, such as PATH and Path.


I am not sure if there is a guarantee that windows always upper cases the keys, however nodejs will chose the upper-cased one in case there are multiple ones. So this should work.

* Upper case electron token env variable key to avoid problems on win

Signed-off-by: Johannes Faltermeier <jfaltermeier@eclipsesource.com>
@paul-marechal paul-marechal merged commit d8064d1 into master Apr 28, 2022
@github-actions github-actions bot added this to the 1.25.0 milestone Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Theia launched from Theia hangs on Windows [Electron]
3 participants