pywlroots icon indicating copy to clipboard operation
pywlroots copied to clipboard

expose xwayland.Server's pid

Open tych0 opened this issue 1 year ago • 3 comments

wlroots wants to fork() and waitpid() on the result:

https://gitlab.freedesktop.org/wlroots/wlroots/-/commit/871646d22522141c45db2c0bfa1528d595bb69df

(this code almost does the right thing, but only does SIG_BLOCK in the first fork, not the parent...)

applications using wlroots which have SIGCHLD handlers installed may interfere with this, so expose the server pid so they can use WNOWAIT trickery to avoid reaping this pid, to allow wlroots to do so.

Maybe this is the wrong patch to fix the described issue: maybe pywayland should block SIGCHLD (via signal.pthread_sigmask(), since applications may be multithreaded, especially if they use asyncio) and then do wlr_xwayland_server_create(), then unblock it for users? But either way, seems like people might want to see the pid here.

tych0 avatar Nov 27 '24 03:11 tych0

That is what labwc is doing as well, it needs to clean up processes it spawns itself (+ get notified when a primary client terminates) and thus has a global SIGCHLD handler. That indeed breaks lazy initialization of xwayland and therefore labwc uses WNOWAIT to peek at the pid and compares it with the xwayland pid before reaping.

Consolatis avatar Nov 27 '24 04:11 Consolatis

Yeah, and actually doing the ptrace_setsigmask() trick won't work, because then the SICHLD will be delivered somewhere else. It really feels like wlroots shouldn't fork here in favor of posix_spawn() or something.

tych0 avatar Nov 27 '24 14:11 tych0

Oh... it turns out they already do a pipe to notify, and the waitpid() that causes the error is really just about zombie prevention, I think. I sent them a patch: https://gitlab.freedesktop.org/wlroots/wlroots/-/merge_requests/4926

tych0 avatar Nov 27 '24 15:11 tych0