feat: Add logout cleanup #606
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
|
Hi @calsys456. Thanks for your PR. I'm waiting for a linuxdeepin member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Reviewer's GuideThis PR refactors session cleanup on logout by centralizing session removal in Helper, enhancing automatic resource cleanup via a custom shared_ptr deleter, updating interface declarations, and ensuring WSocket, WXWayland, and associated surfaces are properly destroyed when a session ends. Sequence diagram for session cleanup on user logoutsequenceDiagram
actor User
participant GreeterProxy
participant Helper
participant "ShellHandler"
participant "RootSurfaceContainer"
participant "WSocket"
participant "WXWayland"
User->>GreeterProxy: Initiate logout()
GreeterProxy->>Helper: removeSession(activeSession)
Helper->>"RootSurfaceContainer": Destroy surfaces for session
Helper->>"ShellHandler": removeXWayland(xwayland)
Helper->>WSocket: Delete WSocket
Helper->>WXWayland: Delete WXWayland
Helper->>Helper: Remove session from session list
Helper->>Helper: Reset activeSession
Helper->>GreeterProxy: Logout cleanup complete
Class diagram for updated Helper session managementclassDiagram
class Helper {
+removeSession(std::shared_ptr<Session> session)
+activeSession() const : std::weak_ptr<Session>
+sessionForUid(uid_t uid) const : std::shared_ptr<Session>
+sessionForXWayland(WXWayland *xwayland) const : std::shared_ptr<Session>
+sessionForSocket(WSocket *socket) const : std::shared_ptr<Session>
-ensureSession(uid_t uid) : std::shared_ptr<Session>
-m_activeSession : std::weak_ptr<Session>
-m_sessions : QList<std::shared_ptr<Session>>
}
class Session {
+uid : uid_t
+socket : WSocket*
+xwayland : WXWayland*
}
Helper "1" *-- "*" Session : manages
Class diagram for ShellHandler XWayland removalclassDiagram
class ShellHandler {
+removeXWayland(WXWayland *xwayland)
-m_xwaylands : QList<WXWayland*>
}
class WXWayland {
}
class WServer {
+detach(WXWayland *xwayland)
+from(WXWayland *xwayland) : WServer*
}
ShellHandler "*" o-- "*" WXWayland : manages
WXWayland "1" --o "1" WServer : attached to
File-Level Changes
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 there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/seat/helper.cpp:2137-2146` </location>
<code_context>
- // Session does not exist, create new session
- auto session = std::make_shared<Session>();
+ // Session does not exist, create new session with deleter
+ std::shared_ptr<Session> session(new Session, [this](Session *s) {
+ qCDebug(treelandCore) << "Deleting session for uid:" << s->uid << s->socket;
+
+ const auto surfaces = m_rootSurfaceContainer->surfaces();
+ for (auto surface : surfaces) {
+ auto client = surface->shellSurface()->waylandClient();
+ if (client && client->socket()->rootSocket() == s->socket) {
+ onSurfaceWrapperAboutToRemove(surface);
+ m_rootSurfaceContainer->destroyForSurface(surface);
+ }
+ }
+
+ m_shellHandler->removeXWayland(s->xwayland);
+ delete s->socket;
+ delete s;
+ });
session->uid = uid;
</code_context>
<issue_to_address>
**issue (bug_risk):** Custom deleter logic may lead to double deletion if session is removed elsewhere.
If other code paths, such as removeSession, also delete s or s->socket, this could cause double deletion. Please clarify and document ownership to prevent undefined behavior.
</issue_to_address>
### Comment 2
<location> `src/seat/helper.cpp:2156-2023` </location>
<code_context>
session->socket = createWSocket();
- if (!session->socket) {
+ if (!session->socket)
return nullptr;
- }
</code_context>
<issue_to_address>
**issue (bug_risk):** Returning nullptr without cleanup may leak the Session object.
If socket creation fails, Session is not deleted, leading to a memory leak. Use a unique_ptr for Session until all allocations succeed, then transfer ownership as needed.
</issue_to_address>
### Comment 3
<location> `src/seat/helper.cpp:2160-2023` </location>
<code_context>
session->xwayland = createXWayland(session->socket);
- if (!session->xwayland) {
- delete session->socket;
+ if (!session->xwayland)
return nullptr;
- }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Potential memory leak if xwayland creation fails after socket allocation.
Ensure session->socket is deleted if createXWayland fails to prevent a memory leak.
</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
Oct 28, 2025
zccrs
requested changes
Oct 28, 2025
|
TAG Bot New tag: 0.7.6 |
calsys456
force-pushed
the
multiuser
branch
2 times, most recently
from
a53a339 to
232deb3
Compare
Collaborator
Author
|
Related with linuxdeepin/ddm#47 |
|
TAG Bot New tag: 0.7.7 |
Delete current session when user logged out, remove all surfaces belongs to the session, and delete WSocket & WXWayland instances.
wl_client should be destroyed together with WClient, to prevent potential error when bare wl_client is applied to globalFilter. It can be done by calling wl_client_destroy in WSocket::removeClient. However, for wl_client created by wlr_xwayland_server, their life cycle is managed by wlroots. Calling wl_client_destroy ourselves will leads to an error, when wlroots trying to destroy it again laterly. Thus we add a new argument noAutoDestroy, which is used to mark WClient created by wlr_xwayland_server, and exclude them in destruction.
In this commit we connect Helper::Session with corresponding org.freedesktop.login1.Session And we fixed some misbehaviour btw.
zccrs
requested changes
Nov 10, 2025
zccrs
requested changes
Nov 11, 2025
calsys456
force-pushed
the
multiuser
branch
3 times, most recently
from
9f71948 to
eb03273
Compare
zccrs
approved these changes
Nov 12, 2025
zccrs
approved these changes
Nov 12, 2025
|
[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.
Delete current session when user logged out, remove all surfaces belongs to the session, and delete WSocket & WXWayland instances.
Summary by Sourcery
Ensure proper cleanup of user sessions by deleting the session, its surfaces, sockets, and XWayland instances upon logout and when sessions are removed.
New Features:
Enhancements: