Skip to content

fix: start:wayland defaults to wrong compositor socket - #4268

Open
flightlesstux wants to merge 1 commit into
MagicMirrorOrg:developfrom
flightlesstux:upstream-wayland-fix
Open

flightlesstux wants to merge 1 commit into
MagicMirrorOrg:developfrom
flightlesstux:upstream-wayland-fix

Conversation

@flightlesstux

Copy link
Copy Markdown

WAYLAND_DISPLAY falls back to wayland-1 when it's not set. On my Pi the real socket is wayland-0, so electron connected to nothing and crashed on every launch. Since pm2 auto-restarts the process, this turned into a crash loop that filled my disk with core dumps and broke git, npm and pm2 along with it.

This checks XDG_RUNTIME_DIR for the actual wayland-* socket and only falls back to wayland-0 if nothing is found.

Tested manually with a socket present, no socket, and WAYLAND_DISPLAY already set. Ran node --run lint:fix, no issues.

WAYLAND_DISPLAY fell back to wayland-1 when unset, but that's not
the real socket on every setup (wayland-0 on mine). With a saved
pm2 env that never set WAYLAND_DISPLAY, electron connected to the
wrong socket and SIGSEGV'd on every launch, and pm2's auto-restart
turned that into a crash loop.

Now it checks XDG_RUNTIME_DIR for the actual wayland-* socket first
and only falls back to wayland-0 if it can't find one.
Comment thread package.json
"start": "node --run start:wayland",
"start:dev": "node --run start:wayland -- dev",
"start:wayland": "WAYLAND_DISPLAY=\"${WAYLAND_DISPLAY:=wayland-1}\" ./node_modules/.bin/electron js/electron.js --ozone-platform=wayland",
"start:wayland": "WAYLAND_DISPLAY=\"${WAYLAND_DISPLAY:-$(ls \"${XDG_RUNTIME_DIR:-/run/user/$(id -u)}\" 2>/dev/null | grep -m1 '^wayland-[0-9]*$')}\"; WAYLAND_DISPLAY=\"${WAYLAND_DISPLAY:-wayland-0}\" ./node_modules/.bin/electron js/electron.js --ozone-platform=wayland",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could this be simplified to the following?

Suggested change
"start:wayland": "WAYLAND_DISPLAY=\"${WAYLAND_DISPLAY:-$(ls \"${XDG_RUNTIME_DIR:-/run/user/$(id -u)}\" 2>/dev/null | grep -m1 '^wayland-[0-9]*$')}\"; WAYLAND_DISPLAY=\"${WAYLAND_DISPLAY:-wayland-0}\" ./node_modules/.bin/electron js/electron.js --ozone-platform=wayland",
"start:wayland": "./node_modules/.bin/electron js/electron.js --ozone-platform=wayland",

The current script explicitly defaults WAYLAND_DISPLAY to wayland-1, while the standard Wayland default is wayland-0. On my Debian system, removing the explicit assignment works correctly.

I haven't tested this on a Raspberry Pi, though. Could someone confirm that Electron also uses wayland-0 there when WAYLAND_DISPLAY is unset?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

works with unset WAYLAND_DISPLAY on a pi 4:

####  System Information  ####
- MM:       version: v2.38.0-develop; git: e1cc0696; branch: develop
- SYSTEM:   manufacturer: Raspberry Pi Foundation; model: Raspberry Pi 4 Model B Rev 1.5; virtual: false
- OS:       platform: linux; distro: Debian GNU/Linux; release: 13; arch: arm64; kernel: 6.18.50+rpt-rpi-v8
- VERSIONS: electron: 44.1.1; used node: 24.19.0; installed node: 22.22.2; npm: 10.9.7; pm2: 
- ENV:      XDG_SESSION_TYPE: tty; MM_CONFIG_FILE: undefined
            WAYLAND_DISPLAY:  undefined; DISPLAY: undefined; ELECTRON_ENABLE_GPU: undefined
- RAM:      total: 3795.74 MB; free: 3183.61 MB; used: 612.13 MB
- OTHERS:   uptime: 18 minutes; timeZone: Europe/Berlin

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@sdetweil What do you think about the simplified version "./node_modules/.bin/electron js/electron.js --ozone-platform=wayland" ?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

is that a temporary parameter? or do you think it will last?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Good question! I just tested this with Electron 44.1.1 on Debian, and MagicMirror started successfully without --ozone-platform=wayland.

Could we simplify the script even further to this?

"start:wayland": "./node_modules/.bin/electron js/electron.js"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

no, this fails on my pi 4 ...

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

mm@pi4-argon:~/MagicMirror $ env | grep -i xdg_
XDG_SESSION_TYPE=tty
XDG_SESSION_CLASS=user
XDG_SESSION_ID=4
XDG_RUNTIME_DIR=/run/user/1000

Running XDG_SESSION_TYPE=wayland ./node_modules/.bin/electron js/electron.js works. Electron uses XDG_SESSION_TYPE if --ozone-platform=auto (the default).

So we should not remove --ozone-platform=wayland.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Okay, thanks for testing! So "./node_modules/.bin/electron js/electron.js --ozone-platform=wayland" stays the candidate.

Out of curiosity: Why do you have XDG_SESSION_TYPE=tty and not XDG_SESSION_TYPE: wayland?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this is the default in raspberry pi os

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@flightlesstux Please confirm if "start:wayland": "./node_modules/.bin/electron js/electron.js --ozone-platform=wayland" also works for you.

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.

4 participants