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

Require an explicit unlock call in SessionLock #443

Merged
merged 1 commit into from
Mar 1, 2024

Conversation

danieldg
Copy link
Contributor

The entire reason unlock_and_destroy is distinct from destroy is to avoid accidental unlock operations. The current SCTK implementation carefully undoes this work by unlocking on drop, even if that drop is due to a panic or careless drop of the lock object.

Add an explicit unlock call so that screen lockers can indicate when they intend to unlock (generally after some kind of authentication has happened).

The entire reason unlock_and_destroy is distinct from destroy is to
avoid accidental unlock operations.  The current SCTK implementation
carefully undoes this work by unlocking on drop, even if that drop is
due to a panic or careless drop of the lock object.

Add an explicit unlock call so that screen lockers can indicate when
they intend to unlock (generally after some kind of authentication has
happened).
Copy link
Member

@wash2 wash2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see why it would be a problem if unlock is called automatically when the application panics.

@ids1024
Copy link
Member

ids1024 commented Mar 1, 2024

The ideal solution would be to have some kind of "linear typing" which would prevent implicit dropping except though panics. But yeah, this is probably correct without any better solution.

This is a breaking API change for any client using session lock.

@wash2 wash2 merged commit ace6907 into Smithay:master Mar 1, 2024
4 of 5 checks passed
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.

3 participants