fix: fix crash on logout #675
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The reason of crash is that the treeland cannot find fullServerName for default WSocket. We should provide an always-available WSocket in defaultWaylandSocket() method, instead of the socket of current session.
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts Helper’s Wayland/XWayland accessors to be const-correct and changes the "default" Wayland socket to a reserved, always-available socket for the fixed user instead of the active session’s socket, preventing crashes when logging out. Sequence diagram for resolving the default Wayland socket on logoutsequenceDiagram
actor Compositor
participant Helper
participant C_Library as C_Library_getpwnam
participant SessionManager as Session_manager
participant Session
participant WSocket
Compositor->>Helper: defaultWaylandSocket()
activate Helper
Helper->>C_Library: getpwnam(dde)
C_Library-->>Helper: passwd_struct_with_uid
Helper->>Helper: waylandSocketForUid(uid)
Helper->>SessionManager: sessionForUid(uid)
SessionManager-->>Helper: Session
Helper->>Session: access socket
Session-->>Helper: WSocket
Helper-->>Compositor: WSocket
deactivate Helper
Class diagram for Helper and session socket accessorsclassDiagram
class Helper {
+addSocket(socket WSocket*) void
+removeXWayland(xwayland WXWayland*) void
+removeSession(session std_shared_ptr_Session) void
+xwaylandForUid(uid uid_t) WXWayland* const
+waylandSocketForUid(uid uid_t) WSocket* const
+sessionForUid(uid uid_t) std_shared_ptr_Session const
+sessionForXWayland(xwayland WXWayland*) std_shared_ptr_Session const
+sessionForSocket(socket WSocket*) std_shared_ptr_Session const
+defaultWaylandSocket() WSocket* const
-m_activeSession std_weak_ptr_Session
}
class Session {
+xwayland WXWayland*
+socket WSocket*
}
class WSocket
class WXWayland
Helper "1" --> "*" Session : manages
Session "1" --> "1" WSocket : socket
Session "1" --> "1" WXWayland : xwayland
Helper ..> WSocket : returns
Helper ..> WXWayland : returns
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
defaultWaylandSocket(), callinggetpwnam("dde")->pw_uidwithout checking for a null return fromgetpwnamcan itself cause a crash; add a null check and a safe fallback path if the user does not exist. - Relying on a hard‑coded username "dde" in
defaultWaylandSocket()may be brittle in customized deployments; consider making this user configurable or deriving it from existing configuration/state instead of hard‑coding. - If
Helperis used from multiple threads, the use ofgetpwnam(which is not thread‑safe) indefaultWaylandSocket()could introduce data races; consider usinggetpwnam_ror resolving and caching the UID at startup.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `defaultWaylandSocket()`, calling `getpwnam("dde")->pw_uid` without checking for a null return from `getpwnam` can itself cause a crash; add a null check and a safe fallback path if the user does not exist.
- Relying on a hard‑coded username "dde" in `defaultWaylandSocket()` may be brittle in customized deployments; consider making this user configurable or deriving it from existing configuration/state instead of hard‑coding.
- If `Helper` is used from multiple threads, the use of `getpwnam` (which is not thread‑safe) in `defaultWaylandSocket()` could introduce data races; consider using `getpwnam_r` or resolving and caching the UID at startup.
## Individual Comments
### Comment 1
<location> `src/seat/helper.cpp:2382` </location>
<code_context>
- if (ptr && ptr->socket)
- return ptr->socket;
- return nullptr;
+ return waylandSocketForUid(getpwnam("dde")->pw_uid);
}
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard against `getpwnam("dde")` returning `nullptr` before dereferencing.
If `getpwnam("dde")` returns `nullptr` (e.g. user missing or misconfigured), this will dereference a null pointer and crash. Store the result in a local variable, check for `nullptr`, and return `nullptr` or another fallback if the lookup fails.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
zccrs
requested changes
Jan 4, 2026
zccrs
approved these changes
Jan 4, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: calsys456, zccrs The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The reason of crash is that the treeland cannot find fullServerName for default WSocket. We should provide an always-available WSocket in defaultWaylandSocket() method, instead of the socket of current session.
Summary by Sourcery
Ensure the default Wayland socket lookup uses a reserved, always-available socket instead of the active session socket to prevent crashes on logout.
Bug Fixes:
Enhancements: