Fix login deadlocks

Logins seem to spuriously fail with both correct or incorrect passwords.
The failure is not related to the password. `cosmic-greeter` hangs but
the GUI still works. The password text area disappears as well.

I traced this issue down to the socket. It seems like accessing the
socket deadlocks with one thread waiting for the socket to become
readable while another waits for it to become writable.

Switching to `greet-ipc`'s `TokioCodec` and adding a lock to the socket
seems to have fixed this issue. I successfully logged in and inputted
incorrect passwords consecutively without experiencing a deadlock.
This commit is contained in:
Josh Megnauth 2024-06-03 00:33:13 -04:00 committed by Jeremy Soller
parent e2a4ccc8bd
commit bba692eecb
3 changed files with 61 additions and 73 deletions

2
Cargo.lock generated
View file

@ -2150,9 +2150,11 @@ version = "0.10.0"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "1600ad23798daf53f5c336ebca8a7c603696ef4455103e9c713fab574131eb35" checksum = "1600ad23798daf53f5c336ebca8a7c603696ef4455103e9c713fab574131eb35"
dependencies = [ dependencies = [
"async-trait",
"serde", "serde",
"serde_json", "serde_json",
"thiserror", "thiserror",
"tokio",
] ]
[[package]] [[package]]

View file

@ -37,7 +37,7 @@ rust-embed = "8"
[dependencies.greetd_ipc] [dependencies.greetd_ipc]
version = "0.10.0" version = "0.10.0"
features = ["sync-codec"] features = ["tokio-codec"]
[features] [features]
default = ["logind", "networkmanager", "upower"] default = ["logind", "networkmanager", "upower"]

View file

@ -24,7 +24,7 @@ use cosmic::{
style, widget, Element, style, widget, Element,
}; };
use cosmic_greeter_daemon::{UserData, WallpaperData}; use cosmic_greeter_daemon::{UserData, WallpaperData};
use greetd_ipc::{codec::SyncCodec, AuthMessageType, Request, Response}; use greetd_ipc::{codec::TokioCodec, AuthMessageType, Request, Response};
use std::{ use std::{
collections::HashMap, collections::HashMap,
env, env,
@ -35,6 +35,7 @@ use std::{
sync::Arc, sync::Arc,
time::{Duration, Instant}, time::{Duration, Instant},
}; };
use tokio::sync::Mutex;
use tokio::{net::UnixStream, time}; use tokio::{net::UnixStream, time};
use wayland_client::{protocol::wl_output::WlOutput, Proxy}; use wayland_client::{protocol::wl_output::WlOutput, Proxy};
use zbus::{proxy, Connection}; use zbus::{proxy, Connection};
@ -272,83 +273,68 @@ pub fn main() -> Result<(), Box<dyn Error>> {
Ok(()) Ok(())
} }
async fn request_message(socket: Arc<UnixStream>, request: Request) -> Message { async fn request_message(socket: Arc<Mutex<UnixStream>>, request: Request) -> Message {
//TODO: handle errors //TODO: handle errors
socket.writable().await.unwrap(); let response = {
{ let mut socket = socket.lock().await;
let mut bytes = Vec::<u8>::new(); request.write_to(&mut *socket).await.unwrap();
request.write_to(&mut bytes).unwrap(); Response::read_from(&mut *socket).await
socket.try_write(&bytes).unwrap(); };
}
//TODO: handle responses at any time? //TODO: handle responses at any time?
loop { match response {
socket.readable().await.unwrap(); Ok(response) => {
log::info!("{:?}", response);
let mut bytes = Vec::<u8>::with_capacity(4096); match response {
match socket.try_read_buf(&mut bytes) { Response::AuthMessage {
Ok(0) => break, auth_message_type,
Ok(_count) => { auth_message,
let mut cursor = io::Cursor::new(bytes); } => match auth_message_type {
let response = Response::read_from(&mut cursor).unwrap(); AuthMessageType::Secret => {
log::info!("{:?}", response); return Message::Prompt(auth_message, true, Some(String::new()));
match response {
Response::AuthMessage {
auth_message_type,
auth_message,
} => match auth_message_type {
AuthMessageType::Secret => {
return Message::Prompt(auth_message, true, Some(String::new()));
}
AuthMessageType::Visible => {
return Message::Prompt(auth_message, false, Some(String::new()));
}
//TODO: treat error type differently?
AuthMessageType::Info | AuthMessageType::Error => {
return Message::Prompt(auth_message, false, None);
}
},
Response::Error {
error_type: _,
description,
} => {
//TODO: use error_type?
return Message::Error(socket, description);
} }
Response::Success => match request { AuthMessageType::Visible => {
Request::CreateSession { .. } => { return Message::Prompt(auth_message, false, Some(String::new()));
// User has no auth required, proceed to login }
return Message::Login(socket); //TODO: treat error type differently?
} AuthMessageType::Info | AuthMessageType::Error => {
Request::PostAuthMessageResponse { .. } => { return Message::Prompt(auth_message, false, None);
// All auth is completed, proceed to login }
return Message::Login(socket); },
} Response::Error {
Request::StartSession { .. } => { error_type: _,
// Session has been started, exit greeter description,
return Message::Exit; } => {
} //TODO: use error_type?
Request::CancelSession => { return Message::Error(socket, description);
// Reconnect to socket
return Message::Reconnect;
}
},
} }
Response::Success => match request {
Request::CreateSession { .. } => {
// User has no auth required, proceed to login
return Message::Login(socket);
}
Request::PostAuthMessageResponse { .. } => {
// All auth is completed, proceed to login
return Message::Login(socket);
}
Request::StartSession { .. } => {
// Session has been started, exit greeter
return Message::Exit;
}
Request::CancelSession => {
// Reconnect to socket
return Message::Reconnect;
}
},
} }
Err(err) => match err.kind() {
io::ErrorKind::WouldBlock => continue,
_ => {
log::error!("failed to read socket: {:?}", err);
break;
}
},
} }
Err(err) => log::error!("failed to read socket: {:?}", err),
} }
Message::None Message::None
} }
fn request_command(socket: Arc<UnixStream>, request: Request) -> Command<Message> { fn request_command(socket: Arc<Mutex<UnixStream>>, request: Request) -> Command<Message> {
Command::perform( Command::perform(
async move { message::app(request_message(socket, request).await) }, async move { message::app(request_message(socket, request).await) },
|x| x, |x| x,
@ -367,7 +353,7 @@ pub enum SocketState {
/// Opening GREETD_SOCK /// Opening GREETD_SOCK
Pending, Pending,
/// GREETD_SOCK is open /// GREETD_SOCK is open
Open(Arc<UnixStream>), Open(Arc<Mutex<UnixStream>>),
/// No GREETD_SOCK variable set /// No GREETD_SOCK variable set
NotSet, NotSet,
/// Failed to open GREETD_SOCK /// Failed to open GREETD_SOCK
@ -403,10 +389,10 @@ pub enum Message {
PowerInfo(Option<(String, f64)>), PowerInfo(Option<(String, f64)>),
Prompt(String, bool, Option<String>), Prompt(String, bool, Option<String>),
Session(String), Session(String),
Username(Arc<UnixStream>, String), Username(Arc<Mutex<UnixStream>>, String),
Auth(Arc<UnixStream>, Option<String>), Auth(Arc<Mutex<UnixStream>>, Option<String>),
Login(Arc<UnixStream>), Login(Arc<Mutex<UnixStream>>),
Error(Arc<UnixStream>, String), Error(Arc<Mutex<UnixStream>>, String),
DialogCancel, DialogCancel,
DialogConfirm, DialogConfirm,
Reconnect, Reconnect,
@ -726,7 +712,7 @@ impl cosmic::Application for App {
async { async {
message::app(Message::Socket(match env::var_os("GREETD_SOCK") { message::app(Message::Socket(match env::var_os("GREETD_SOCK") {
Some(socket_path) => match UnixStream::connect(&socket_path).await { Some(socket_path) => match UnixStream::connect(&socket_path).await {
Ok(socket) => SocketState::Open(Arc::new(socket)), Ok(socket) => SocketState::Open(Arc::new(socket.into())),
Err(err) => SocketState::Error(Arc::new(err)), Err(err) => SocketState::Error(Arc::new(err)),
}, },
None => SocketState::NotSet, None => SocketState::NotSet,