Skip to content

Commit

Permalink
Require an explicit unlock call in SessionLock
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
danieldg authored and wash2 committed Mar 1, 2024
1 parent f3587a9 commit ace6907
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 10 deletions.
4 changes: 2 additions & 2 deletions examples/session_lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ impl SessionLockHandler for AppData {
// After 5 seconds, destroy lock
self.loop_handle
.insert_source(Timer::from_duration(Duration::from_secs(5)), |_, _, app_data| {
// Destroy the session lock
app_data.session_lock.take();
// Unlock the lock
app_data.session_lock.take().unwrap().unlock();
// Sync connection to make sure compostor receives destroy
app_data.conn.roundtrip().unwrap();
// Then we can exit
Expand Down
21 changes: 13 additions & 8 deletions src/session_lock/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,19 +89,18 @@ pub struct SessionLockInner {

impl Drop for SessionLockInner {
fn drop(&mut self) {
if self.locked.load(Ordering::SeqCst) {
self.session_lock.unlock_and_destroy();
} else {
self.session_lock.destroy();
}
// This does nothing if unlock() was called. It may trigger a protocol error if unlock was
// not called; this is an application bug, and choosing not to unlock here results in us
// failing secure.
self.session_lock.destroy();
}
}

/// A session lock
///
/// The lock is destroyed on drop, which must be done after `locked` or `finished`
/// is received.
#[must_use]
/// Once a lock is created, you must wait for either a `locked` or `finished` event before
/// destroying this object. If you get a `locked` event, you must explicitly call `unlock` prior
/// to dropping this object.
#[derive(Debug, Clone)]
pub struct SessionLock(Arc<SessionLockInner>);

Expand All @@ -113,6 +112,12 @@ impl SessionLock {
pub fn is_locked(&self) -> bool {
self.0.locked.load(Ordering::SeqCst)
}

pub fn unlock(&self) {
if self.0.locked.load(Ordering::SeqCst) {
self.0.session_lock.unlock_and_destroy();
}
}
}

#[derive(Debug)]
Expand Down

0 comments on commit ace6907

Please sign in to comment.