-
Notifications
You must be signed in to change notification settings - Fork 16
fix: fix user cannot login after logout once #63
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
Conversation
We manually stop the auth before terminating logind session, so that the session can be closed properly by PAM without being left in an undetermined state.
Reviewer's guide (collapsed on small PRs)Reviewer's GuideEnsures user sessions are cleanly stopped and logged out by explicitly stopping the associated Auth before terminating the logind session, and by reusing existing environment data instead of querying the child process environment after it may have been stopped. Sequence diagram for logout flow with explicit Auth stop before logind TerminateSessionsequenceDiagram
actor User
participant Display
participant Auth
participant SessionProcess as SessionProcess
participant PAM as PAMSession
participant Logind as LogindManager
participant Utmp as UtmpLogger
User ->> Display: requestLogout(id)
activate Display
Display ->> Display: logout(id)
loop for each auth in auths
Display ->> Auth: check xdgSessionId
alt matching xdgSessionId
Display ->> Auth: stop()
activate Auth
Auth ->> SessionProcess: stop()
SessionProcess -->> Auth: processStopped
Auth ->> PAM: closeSession()
PAM -->> Auth: sessionClosed
Auth ->> Utmp: utmpLogout(vt, displayId, pid)
Utmp -->> Auth: logoutRecorded
deactivate Auth
end
end
Display ->> Logind: TerminateSession(id)
Logind -->> Display: sessionTerminated
deactivate Display
Class diagram for updated Auth and Display logout behaviorclassDiagram
class Auth {
+QString xdgSessionId
-QProcess *m_session
-QProcessEnvironment m_env
-PamHandle *m_pam
+void stop()
}
class Display {
-QList~Auth*~ auths
+void logout(QLocalSocket *socket, int id)
}
class PamHandle {
+bool sessionOpened
+void closeSession()
}
class QProcessEnvironment {
+QString value(QString name)
}
class QProcess {
+qint64 processId()
+QProcessEnvironment processEnvironment()
+QProcess::ProcessState state()
+void stop()
}
Auth --> QProcess : uses m_session
Auth --> QProcessEnvironment : uses m_env
Auth --> PamHandle : uses m_pam
Display "1" o-- "*" Auth : manages auths
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've left some high level feedback:
- In
Display::logout, once you find anAuthwith a matchingxdgSessionIdand callstop(), you canbreakout of the loop to avoid redundantstop()calls in the unlikely event of multiple matches and make the intent clearer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `Display::logout`, once you find an `Auth` with a matching `xdgSessionId` and call `stop()`, you can `break` out of the loop to avoid redundant `stop()` calls in the unlikely event of multiple matches and make the intent clearer.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
[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 |
We manually stop the auth before terminating logind session, so that the session can be closed properly by PAM without being left in an undetermined state.
Summary by Sourcery
Ensure user sessions are cleanly stopped before terminating the corresponding logind session to prevent login issues after logout.
Bug Fixes:
Enhancements: