Skip to content

Commit

Permalink
Fix rust advlogger deadlock (#356)
Browse files Browse the repository at this point in the history
## Description

This PR resolves an issue where invocation of the debugln!() macro could
result in deadlock due to contention between two different TPL levels.
The deadlock occurs in the following scenario:

1. A task running at a lower TPL invokes debugln!() and acquires the
spinlock on the logger object.
2. That task is interrupted by a TPL running at a higher level (the
lower task has not released the lock).
3. The higher TPL invokes debugln!(). 

In this scenario, the higher TPL task cannot make forward progress
because it cannot acquire the lock held by the lower TPL task, and the
lower TPL task is not executing because it was interrupted by the higher
TPL task.

This resolves the issue by changing the "lock" to a "try_lock" - in the
scenario above, this allows the higher TPL task to make forward
progress. This has the downside of dropping the message from the higher
TPL task; so this is only intended as an interim fix.

- [ ] Impacts functionality?
- [ ] Impacts security?
- [ ] Breaking change?
- [ ] Includes tests?
- [ ] Includes documentation?

## How This Was Tested

Reproduced the issue using QEMU emulator; with this change the above
flow no longer deadlocks.

## Integration Instructions

N/A
  • Loading branch information
joschock committed Nov 15, 2023
1 parent feed557 commit 1edf727
Showing 1 changed file with 6 additions and 1 deletion.
7 changes: 6 additions & 1 deletion AdvLoggerPkg/Crates/RustAdvancedLoggerDxe/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,12 @@ impl LockedAdvancedLogger {

// Log the debug output in `args` at the given log level.
fn log(&self, level: usize, args: fmt::Arguments) {
self.inner.lock().log(level, args)
// Note: tasks at higher TPL may interrupt logging of tasks at lower TPL. This could cause deadlock here, if the
// lower TPL thread is holding the lock and is interrupted at a higher TPL. For now, use try_lock() to avoid
// deadlock here. This has the downside of potentially dropping messages at higher TPL.
if let Some(mut logger) = self.inner.try_lock() {
logger.log(level, args)
}
}
}

Expand Down

0 comments on commit 1edf727

Please sign in to comment.