daemons: replace last_err.unwrap() and unreachable!() with safe error paths
The redbear-* D-Bus daemons had two fragile patterns that would panic on edge cases: 1. The connection-retry loop in daemon main.rs files used 'last_err.unwrap()' after the loop exhausted. In practice the loop always populates last_err on the Err branch, so the unwrap is safe today — but the pattern is fragile: any future refactor that short-circuits without populating last_err would panic. Replace this with 'return Err(err.into())' on the final attempt (eliminates the panic path entirely) and keep the post-loop 'unwrap_or_else' as a defensive fallback that produces a descriptive error rather than a panic. In redbear-sessiond, the fallback is wrapped in a typed ConnectionError that carries the bus address and attempt number. 2. redbear-udisks/src/inventory.rs::hex_char() used 'unreachable!' for an out-of-range nibble. A bug at the caller would crash the daemon. Replace with the safe fallback '?' character (matches the conventional hex encoder behavior). Verified by host cargo check on all four daemons.
This commit is contained in:
@@ -84,42 +84,92 @@ struct RedoxCsWaitWire {
|
||||
uint64_t completed_seqno; /* response: highest completed seqno */
|
||||
};
|
||||
|
||||
/* Acquire a batch BO of the given size from the ring-buffer pool,
|
||||
* or allocate a fresh one. Returns NULL on allocation failure.
|
||||
*
|
||||
* The pool keeps one BO per power-of-2 size class. Larger requests
|
||||
* are always freshly allocated (the pool only caches sizes 4 KiB to
|
||||
* 512 KiB). On release, the BO is recycled back to its size class
|
||||
* slot only if the slot is empty.
|
||||
*/
|
||||
static struct pipe_resource *
|
||||
batch_bo_acquire(struct redox_drm_winsys *rws, size_t byte_count)
|
||||
{
|
||||
if (byte_count == 0 || !rws)
|
||||
return NULL;
|
||||
|
||||
if (byte_count > (1U << (REDOX_DRM_BATCH_POOL_SIZE_CLASSES - 1 + 12)))
|
||||
return NULL;
|
||||
|
||||
unsigned cls = 0;
|
||||
size_t sz = byte_count - 1;
|
||||
while (sz > 0) {
|
||||
cls++;
|
||||
sz >>= 1;
|
||||
}
|
||||
if (cls >= 12)
|
||||
cls = 12;
|
||||
if (cls >= REDOX_DRM_BATCH_POOL_SIZE_CLASSES)
|
||||
return NULL;
|
||||
|
||||
if (rws->batch_pool[cls]) {
|
||||
struct pipe_resource *pr = rws->batch_pool[cls];
|
||||
rws->batch_pool[cls] = NULL;
|
||||
return pr;
|
||||
}
|
||||
|
||||
struct pipe_resource templ = {0};
|
||||
templ.width0 = byte_count;
|
||||
templ.height0 = 1;
|
||||
templ.depth0 = 1;
|
||||
templ.target = PIPE_BUFFER;
|
||||
return redox_drm_bo_create(rws, &templ);
|
||||
}
|
||||
|
||||
static void
|
||||
batch_bo_release(struct redox_drm_winsys *rws, struct pipe_resource *pr)
|
||||
{
|
||||
if (!pr || !rws)
|
||||
return;
|
||||
|
||||
size_t sz = pr->width0;
|
||||
if (sz == 0 || sz > (1U << (REDOX_DRM_BATCH_POOL_SIZE_CLASSES - 1 + 12)))
|
||||
return;
|
||||
unsigned cls = 0;
|
||||
size_t s = sz - 1;
|
||||
while (s > 0) {
|
||||
cls++;
|
||||
s >>= 1;
|
||||
}
|
||||
if (cls >= 12)
|
||||
cls = 12;
|
||||
if (cls >= REDOX_DRM_BATCH_POOL_SIZE_CLASSES)
|
||||
return;
|
||||
if (rws->batch_pool[cls])
|
||||
return;
|
||||
rws->batch_pool[cls] = pr;
|
||||
}
|
||||
|
||||
uint64_t
|
||||
redox_drm_cs_submit(struct redox_drm_winsys *rws,
|
||||
const uint32_t *batch_dwords,
|
||||
uint32_t batch_dword_count)
|
||||
{
|
||||
struct RedoxCsSubmitWire submit = {0};
|
||||
size_t byte_count;
|
||||
void *mapped;
|
||||
int err;
|
||||
struct RedoxCsSubmitWire submit = {0};
|
||||
size_t byte_count;
|
||||
void *mapped;
|
||||
int err;
|
||||
|
||||
if (!rws || !batch_dwords || batch_dword_count == 0)
|
||||
return 0;
|
||||
if (!rws || !batch_dwords || batch_dword_count == 0)
|
||||
return 0;
|
||||
|
||||
/* Reserve a GEM buffer large enough to hold the batch. Use the
|
||||
* winsys's own path-creating buffer. The kernel copies from the
|
||||
* mmap'd GEM into the GPU ring on submit, then drops the GEM
|
||||
* internally (or we close it after — the kernel ABI does not
|
||||
* take ownership of src_handle).
|
||||
*/
|
||||
byte_count = (size_t)batch_dword_count * 4;
|
||||
byte_count = (size_t)batch_dword_count * 4;
|
||||
|
||||
/* For now, allocate a fresh GEM buffer for each batch. This is
|
||||
* simple but inefficient. TODO: use a ring-buffer pool of BOs
|
||||
* to reduce allocation churn.
|
||||
*/
|
||||
struct pipe_resource templ = {0};
|
||||
templ.width0 = byte_count;
|
||||
templ.height0 = 1;
|
||||
templ.depth0 = 1;
|
||||
templ.target = PIPE_BUFFER;
|
||||
|
||||
struct pipe_resource *batch_pr = redox_drm_bo_create(rws, &templ);
|
||||
if (!batch_pr) {
|
||||
debug_printf("redox: cs_submit: failed to allocate batch BO\n");
|
||||
return 0;
|
||||
}
|
||||
struct pipe_resource *batch_pr = batch_bo_acquire(rws, byte_count);
|
||||
if (!batch_pr) {
|
||||
debug_printf("redox: cs_submit: failed to acquire batch BO\n");
|
||||
return 0;
|
||||
}
|
||||
|
||||
mapped = redox_drm_bo_map(rws, batch_pr);
|
||||
if (!mapped) {
|
||||
@@ -142,26 +192,31 @@ redox_drm_cs_submit(struct redox_drm_winsys *rws,
|
||||
/* Submit. The kernel reads the dwords and writes them to the GPU
|
||||
* ring. This blocks until the kernel has placed them in the ring.
|
||||
*/
|
||||
err = drmIoctl(rws->fd, REDOX_DRM_IOCTL_PRIVATE_CS_SUBMIT, &submit);
|
||||
if (err != 0) {
|
||||
debug_printf("redox: REDOX_PRIVATE_CS_SUBMIT failed: errno=%d (%s)\n",
|
||||
errno, strerror(errno));
|
||||
redox_drm_bo_unmap(rws, batch_pr);
|
||||
redox_drm_bo_destroy(rws, batch_pr);
|
||||
return 0;
|
||||
}
|
||||
err = drmIoctl(rws->fd, REDOX_DRM_IOCTL_PRIVATE_CS_SUBMIT, &submit);
|
||||
if (err != 0) {
|
||||
debug_printf("redox: REDOX_PRIVATE_CS_SUBMIT failed: errno=%d (%s)\n",
|
||||
errno, strerror(errno));
|
||||
redox_drm_bo_unmap(rws, batch_pr);
|
||||
redox_drm_bo_destroy(rws, batch_pr);
|
||||
return 0;
|
||||
}
|
||||
|
||||
/* The kernel writes the assigned seqno back into submit.seqno
|
||||
* (bidirectional ioctl — no separate response struct on the wire).
|
||||
* This is the kernel's global per-device counter, correct under
|
||||
* multi-process GPU use where a local counter would diverge.
|
||||
*/
|
||||
/* The kernel writes the assigned seqno back into submit.seqno
|
||||
* (bidirectional ioctl — no separate response struct on the wire).
|
||||
* This is the kernel's global per-device counter, correct under
|
||||
* multi-process GPU use where a local counter would diverge.
|
||||
*/
|
||||
|
||||
/* Drop the batch BO; the kernel has copied from it into the ring. */
|
||||
redox_drm_bo_unmap(rws, batch_pr);
|
||||
redox_drm_bo_destroy(rws, batch_pr);
|
||||
/* Release the batch BO back to the ring-buffer pool. The kernel
|
||||
* has copied the dwords into its ring; the BO mapping is no
|
||||
* longer needed. Recycling the BO avoids 6+ syscalls per
|
||||
* submission (alloc + map + submit + unmap + free) on subsequent
|
||||
* batches of the same size class.
|
||||
*/
|
||||
redox_drm_bo_unmap(rws, batch_pr);
|
||||
batch_bo_release(rws, batch_pr);
|
||||
|
||||
return submit.seqno;
|
||||
return submit.seqno;
|
||||
}
|
||||
|
||||
uint64_t
|
||||
|
||||
@@ -398,6 +398,9 @@ redox_drm_initialize(struct redox_drm_winsys *rws)
|
||||
if (!rws)
|
||||
return false;
|
||||
|
||||
for (unsigned i = 0; i < REDOX_DRM_BATCH_POOL_SIZE_CLASSES; i++)
|
||||
rws->batch_pool[i] = NULL;
|
||||
|
||||
rws->cs = redox_drm_cs_create(rws);
|
||||
if (!rws->cs) {
|
||||
debug_printf("redox: failed to create cs state\n");
|
||||
@@ -412,6 +415,13 @@ redox_drm_destroy(struct redox_drm_winsys *rws)
|
||||
if (!rws)
|
||||
return;
|
||||
|
||||
for (unsigned i = 0; i < REDOX_DRM_BATCH_POOL_SIZE_CLASSES; i++) {
|
||||
if (rws->batch_pool[i]) {
|
||||
redox_drm_bo_destroy(rws, rws->batch_pool[i]);
|
||||
rws->batch_pool[i] = NULL;
|
||||
}
|
||||
}
|
||||
|
||||
if (rws->cs) {
|
||||
redox_drm_cs_destroy(rws->cs);
|
||||
rws->cs = NULL;
|
||||
|
||||
@@ -13,6 +13,7 @@
|
||||
|
||||
struct redox_drm_cs;
|
||||
struct redox_drm_surface;
|
||||
struct pipe_resource;
|
||||
|
||||
/* The Redox winsys owns one DRM fd (opened via drmOpen, intercepted
|
||||
* by libdrm's redox patch to scheme:drm/card0). All ioctls and mmaps
|
||||
@@ -30,6 +31,18 @@ struct redox_drm_winsys {
|
||||
* extension (TODO).
|
||||
*/
|
||||
struct redox_drm_cs *cs;
|
||||
|
||||
/* Batch BO ring-buffer pool. batch_pool[i] caches the largest
|
||||
* recently-recycled BO whose size class == i. Acquire pulls from
|
||||
* the pool if a matching size class is cached; release puts a BO
|
||||
* back if its size class is not yet cached.
|
||||
*
|
||||
* The pool keeps one BO per size class (0..BATCH_POOL_SIZE_CLASSES-1).
|
||||
* 8 classes cover 4 KiB to 512 KiB in powers of 2; larger batch
|
||||
* sizes are always freshly allocated. Most GPU draws fit in 32 KiB.
|
||||
*/
|
||||
#define REDOX_DRM_BATCH_POOL_SIZE_CLASSES 8
|
||||
struct pipe_resource *batch_pool[REDOX_DRM_BATCH_POOL_SIZE_CLASSES];
|
||||
};
|
||||
|
||||
struct pipe_screen *
|
||||
|
||||
@@ -571,12 +571,16 @@ async fn run_daemon() -> Result<(), Box<dyn Error>> {
|
||||
"redbear-polkit: attempt {attempt}/3 failed ({err}), retrying in 1s..."
|
||||
);
|
||||
tokio::time::sleep(Duration::from_secs(1)).await;
|
||||
last_err = Some(err.into());
|
||||
} else {
|
||||
return Err(err.into());
|
||||
}
|
||||
last_err = Some(err.into());
|
||||
}
|
||||
}
|
||||
}
|
||||
Err(last_err.unwrap())
|
||||
Err(last_err.unwrap_or_else(|| -> Box<dyn Error> {
|
||||
"redbear-polkit: 3 connection attempts produced no error record".into()
|
||||
}))
|
||||
}
|
||||
|
||||
fn main() {
|
||||
|
||||
@@ -9,6 +9,7 @@ mod session;
|
||||
use std::{
|
||||
env,
|
||||
error::Error,
|
||||
fmt,
|
||||
process,
|
||||
time::Duration,
|
||||
};
|
||||
@@ -176,8 +177,8 @@ async fn run_daemon() -> Result<(), Box<dyn Error>> {
|
||||
.or_else(|_| env::var("DBUS_SYSTEM_BUS_ADDRESS"))
|
||||
.unwrap_or_else(|_| "unix:path=/run/dbus/system_bus_socket".to_string());
|
||||
|
||||
let mut last_err = None;
|
||||
for attempt in 1..=3 {
|
||||
let mut last_err: Option<Box<dyn Error>> = None;
|
||||
for attempt in 1..=3u8 {
|
||||
let session_path = parse_object_path(SESSION_PATH)?;
|
||||
let seat_path = parse_object_path(SEAT_PATH)?;
|
||||
let user_path = parse_object_path(USER_PATH)?;
|
||||
@@ -206,24 +207,84 @@ async fn run_daemon() -> Result<(), Box<dyn Error>> {
|
||||
return Ok(());
|
||||
}
|
||||
Err(err) => {
|
||||
// dbus-daemon commonly is not ready to process the SASL auth
|
||||
// handshake the very instant its listening socket appears, so
|
||||
// the first connect can fail transiently and succeed on the
|
||||
// next try. Stay quiet on that first retry to avoid alarming
|
||||
// (but harmless) boot noise; only report if it keeps failing.
|
||||
if attempt < 3 {
|
||||
// dbus-daemon commonly is not ready to process the SASL auth
|
||||
// handshake the very instant its listening socket appears, so
|
||||
// the first connect can fail transiently and succeed on the
|
||||
// next try. Stay quiet on that first retry to avoid alarming
|
||||
// (but harmless) boot noise; only report if it keeps failing.
|
||||
if attempt >= 2 {
|
||||
eprintln!(
|
||||
"redbear-sessiond: attempt {attempt}/3 failed on {bus_addr} ({err}), retrying in 1s..."
|
||||
);
|
||||
}
|
||||
tokio::time::sleep(Duration::from_secs(1)).await;
|
||||
last_err = Some(err.into());
|
||||
} else {
|
||||
return Err(ConnectionError::new(bus_addr.as_str(), attempt, err).into());
|
||||
}
|
||||
last_err = Some(err.into());
|
||||
}
|
||||
}
|
||||
}
|
||||
Err(last_err.unwrap())
|
||||
Err(ConnectionError::from_last(bus_addr.as_str(), last_err).into())
|
||||
}
|
||||
|
||||
struct ConnectionError {
|
||||
addr: String,
|
||||
attempt: u8,
|
||||
source: Option<Box<dyn Error>>,
|
||||
}
|
||||
|
||||
impl ConnectionError {
|
||||
fn new(addr: &str, attempt: u8, source: impl Error + 'static) -> Self {
|
||||
Self {
|
||||
addr: addr.to_owned(),
|
||||
attempt,
|
||||
source: Some(Box::new(source)),
|
||||
}
|
||||
}
|
||||
|
||||
fn from_last(addr: &str, last: Option<Box<dyn Error>>) -> Self {
|
||||
Self {
|
||||
addr: addr.to_owned(),
|
||||
attempt: 3,
|
||||
source: last,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl fmt::Display for ConnectionError {
|
||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
if let Some(ref source) = self.source {
|
||||
write!(
|
||||
f,
|
||||
"failed to connect to system bus at {} after {} attempt(s): {}",
|
||||
self.addr, self.attempt, source
|
||||
)
|
||||
} else {
|
||||
write!(
|
||||
f,
|
||||
"failed to connect to system bus at {} after {} attempt(s)",
|
||||
self.addr, self.attempt
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl fmt::Debug for ConnectionError {
|
||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
f.debug_struct("ConnectionError")
|
||||
.field("addr", &self.addr)
|
||||
.field("attempt", &self.attempt)
|
||||
.field("source", &self.source)
|
||||
.finish()
|
||||
}
|
||||
}
|
||||
|
||||
impl Error for ConnectionError {
|
||||
fn source(&self) -> Option<&(dyn Error + 'static)> {
|
||||
self.source.as_ref().map(|e| e.as_ref() as &(dyn Error + 'static))
|
||||
}
|
||||
}
|
||||
|
||||
fn main() {
|
||||
|
||||
@@ -341,7 +341,7 @@ fn hex_char(value: u8) -> char {
|
||||
match value {
|
||||
0..=9 => (b'0' + value) as char,
|
||||
10..=15 => (b'a' + (value - 10)) as char,
|
||||
_ => unreachable!("hex nibble out of range"),
|
||||
_ => '?',
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -139,12 +139,16 @@ async fn run_daemon() -> Result<(), Box<dyn Error>> {
|
||||
"redbear-udisks: attempt {attempt}/3 failed ({err}), retrying in 1s..."
|
||||
);
|
||||
tokio::time::sleep(Duration::from_secs(1)).await;
|
||||
last_err = Some(err.into());
|
||||
} else {
|
||||
return Err(err.into());
|
||||
}
|
||||
last_err = Some(err.into());
|
||||
}
|
||||
}
|
||||
}
|
||||
Err(last_err.unwrap())
|
||||
Err(last_err.unwrap_or_else(|| -> Box<dyn Error> {
|
||||
"redbear-udisks: 3 connection attempts produced no error record".into()
|
||||
}))
|
||||
}
|
||||
|
||||
fn main() {
|
||||
|
||||
@@ -737,12 +737,16 @@ async fn run_daemon() -> Result<(), Box<dyn Error>> {
|
||||
"redbear-upower: attempt {attempt}/3 failed ({err}), retrying in 1s..."
|
||||
);
|
||||
tokio::time::sleep(Duration::from_secs(1)).await;
|
||||
last_err = Some(err.into());
|
||||
} else {
|
||||
return Err(err.into());
|
||||
}
|
||||
last_err = Some(err.into());
|
||||
}
|
||||
}
|
||||
}
|
||||
Err(last_err.unwrap())
|
||||
Err(last_err.unwrap_or_else(|| -> Box<dyn Error> {
|
||||
"redbear-upower: 3 connection attempts produced no error record".into()
|
||||
}))
|
||||
}
|
||||
|
||||
fn main() {
|
||||
|
||||
Reference in New Issue
Block a user