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

steamcompmgr: avoid xwayland server mutex deadlock #1013

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

q66
Copy link

@q66 q66 commented Nov 11, 2023

This uses pthread_exit, which does not ensure C++ destructors are called. That means the steamcompmgr mutex may remain locked forever, which will prevent gamescope from quitting, as wlserver will deadlock on trying to lock it.

Take this strategy: when steamcompmgr_main exits normally, there is no need to call pthread_exit as the thread will stop on its own via returning from its function; this will correctly release all resources.

Upon invocation of error handler however, do call pthread_exit and unlock the mutex manually.

@q66
Copy link
Author

q66 commented Nov 11, 2023

(note that glibc unwinds the stack upon pthread_exit so you will not observe this behavior, however that's a non-portable assumption as there is no guarantee by the specification, and in my case i observe it because of musl libc; unwinding implies linkage to libgcc_s (or libunwind for llvm) which is bad for all sorts of reasons)

This uses pthread_exit, which does not ensure C++ destructors
are called. That means the steamcompmgr mutex may remain locked
forever, which will prevent gamescope from quitting, as wlserver
will deadlock on trying to lock it.

Take this strategy: when steamcompmgr_main exits normally, there
is no need to call pthread_exit as the thread will stop on its
own via returning from its function; this will correctly release
all resources.

Upon invocation of error handler however, do call pthread_exit
and unlock the mutex manually.
@emersion
Copy link
Collaborator

I'd prefer to not add more hacks on top of the existing hacks, that said I have no idea how to properly fix this.

@q66
Copy link
Author

q66 commented Nov 14, 2023

this code is overall kind of a mess, i think some assumptions don't even hold as it currently is - e.g. https://github.com/ValveSoftware/gamescope/blob/master/src/main.cpp#L904 added by 24b5611 will never be reached because of the whole pthread_exit deal

@q66
Copy link
Author

q66 commented Nov 14, 2023

in any case this is not a hack, what's actually a hack is glibc's forced unwinding and the consequences it brings (e.g. dlopening libgcc_s from inside libc)

@q66
Copy link
Author

q66 commented Nov 14, 2023

a proper fix in this case would probably be throwing an exception from the error callback and catching it in steamCompMgrThreadRun, and then not calling any of the pthread_ APIs for this at all; but exception safety in gamescope's code is virtually non-existent

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.

None yet

2 participants