Skip to content

fix: register waker for console poll function - #2756

Open
GloriousAlpaca wants to merge 1 commit into
hermit-os:mainfrom
GloriousAlpaca:fix/console-stdin-poll-waker
Open

GloriousAlpaca wants to merge 1 commit into
hermit-os:mainfrom
GloriousAlpaca:fix/console-stdin-poll-waker

Conversation

@GloriousAlpaca

Copy link
Copy Markdown

Fixes #2748
Before ConsoleStdin::poll() did not register a waker so if no input has been received before calling select(), the thread is stuck, even when receiving input afterwards.

I have edited the poll function to be closer to ConsoleStdin::read(), so it now correctly registers the waker and checks for input when a UART Interrupt is received.

This has fixed the reproducer that did not function before:

#include <stdio.h>
#include <sys/select.h>

int main()
{
    fd_set readfds;

    FD_ZERO(&readfds);
    FD_SET(0, &readfds);
    printf("Select Test: Before select() call\n");
    fflush(stdout);
    int result = select(1, &readfds, (fd_set*)NULL, NULL, NULL);
    printf("Select Test: After select() call, result = %d\n", result);
    return 0;
}

@github-actions github-actions Bot added the fd label Oct 28, 2026

@github-actions github-actions Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Benchmark Results

Details
Benchmark Current: b2097d0 Previous: 2e23902 Performance Ratio
startup_benchmark Build Time 82.90 s 80.34 s 1.03 ❗
startup_benchmark File Size 0.84 MB 0.80 MB 1.05 ❗
Startup Time - 1 core 0.70 s (±0.02 s) 0.75 s (±0.02 s) 0.94 ❗
Startup Time - 2 cores 0.73 s (±0.02 s) 0.74 s (±0.02 s) 0.99
Startup Time - 4 cores 0.74 s (±0.01 s) 0.74 s (±0.02 s) 0.99
multithreaded_benchmark Build Time 84.46 s 82.11 s 1.03 ❗
multithreaded_benchmark File Size 0.94 MB 0.86 MB 1.10 ❗
Multithreaded Pi Efficiency - 2 Threads 92.23 % (±8.64 %) 85.89 % (±6.61 %) 1.07
Multithreaded Pi Efficiency - 4 Threads 44.56 % (±3.71 %) 43.43 % (±2.56 %) 1.03
Multithreaded Pi Efficiency - 8 Threads 26.17 % (±1.64 %) 25.76 % (±1.53 %) 1.02
micro_benchmarks Build Time 84.19 s 80.40 s 1.05 ❗
micro_benchmarks File Size 0.94 MB 0.86 MB 1.10 ❗
Scheduling time - 1 thread 64.60 ticks (±3.39 ticks) 62.65 ticks (±4.06 ticks) 1.03
Scheduling time - 2 threads 36.20 ticks (±3.55 ticks) 34.08 ticks (±4.10 ticks) 1.06
Micro - Time for syscall (getpid) 3.72 ticks (±0.60 ticks) 3.45 ticks (±0.58 ticks) 1.08
Memcpy speed - (built_in) block size 4096 81604.86 MByte/s (±56575.84 MByte/s) 82448.38 MByte/s (±56997.13 MByte/s) 0.99
Memcpy speed - (built_in) block size 1048576 30140.45 MByte/s (±24405.16 MByte/s) 30585.98 MByte/s (±24707.84 MByte/s) 0.99
Memcpy speed - (built_in) block size 16777216 28803.89 MByte/s (±23700.74 MByte/s) 26340.06 MByte/s (±21720.96 MByte/s) 1.09
Memset speed - (built_in) block size 4096 81915.23 MByte/s (±56797.42 MByte/s) 82292.76 MByte/s (±56891.50 MByte/s) 1.00
Memset speed - (built_in) block size 1048576 30858.39 MByte/s (±24822.25 MByte/s) 31323.85 MByte/s (±25145.86 MByte/s) 0.99
Memset speed - (built_in) block size 16777216 29563.62 MByte/s (±24141.28 MByte/s) 27104.68 MByte/s (±22209.94 MByte/s) 1.09
Memcpy speed - (rust) block size 4096 74069.60 MByte/s (±51678.12 MByte/s) 74097.96 MByte/s (±51811.44 MByte/s) 1.00
Memcpy speed - (rust) block size 1048576 30052.47 MByte/s (±24377.01 MByte/s) 30361.60 MByte/s (±24602.37 MByte/s) 0.99
Memcpy speed - (rust) block size 16777216 28913.43 MByte/s (±23816.93 MByte/s) 27625.34 MByte/s (±22806.88 MByte/s) 1.05
Memset speed - (rust) block size 4096 74532.83 MByte/s (±52018.63 MByte/s) 74373.47 MByte/s (±51976.48 MByte/s) 1.00
Memset speed - (rust) block size 1048576 30786.20 MByte/s (±24799.64 MByte/s) 31110.89 MByte/s (±25033.24 MByte/s) 0.99
Memset speed - (rust) block size 16777216 29674.71 MByte/s (±24253.88 MByte/s) 28386.93 MByte/s (±23265.03 MByte/s) 1.05
alloc_benchmarks Build Time 82.02 s 74.76 s 1.10 ❗
alloc_benchmarks File Size 0.92 MB 0.87 MB 1.05 ❗
Allocations - Allocation success 91.37 % 91.31 % 1.00 ❗
Allocations - Deallocation success 100.00 % 100.00 % 1
Allocations - Pre-fail Allocations 61.61 % 61.44 % 1.00 ❗
Allocations - Average Allocation time 6314.74 Ticks (±82.44 Ticks) 5860.58 Ticks (±98.43 Ticks) 1.08 ❗
Allocations - Average Allocation time (no fail) 6880.72 Ticks (±108.38 Ticks) 6554.81 Ticks (±92.86 Ticks) 1.05 ❗
Allocations - Average Deallocation time 1268.95 Ticks (±253.67 Ticks) 1805.01 Ticks (±250.35 Ticks) 0.70 ❗
mutex_benchmark Build Time 81.79 s 79.82 s 1.02 ❗
mutex_benchmark File Size 0.94 MB 0.86 MB 1.10 ❗
Mutex Stress Test Average Time per Iteration - 1 Threads 12.22 ns (±0.46 ns) 12.10 ns (±0.41 ns) 1.01
Mutex Stress Test Average Time per Iteration - 2 Threads 69.24 ns (±3.80 ns) 40.26 ns (±1.68 ns) 1.72 ❗

This comment was automatically generated by workflow using github-action-benchmark.

Comment thread src/fd/stdio/console.rs

Ok(event & available)
future::poll_fn(|cx| {
CONSOLE_WAKER.lock().register(cx.waker());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We only need to register the waker when retuning Pending.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I think there's a possibility of receiving an interrupt between checking for readiness and registering a waker. I'm not sure how high the chance for this to occur is during normal execution, or if there's something in place preventing this?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You are right. We could either check again before returning Pending to optimize the fast path or restore your original proposal. Both are fine with me. Performance should not really be an issue here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I think I'd prefer the original version, as it looks a bit cleaner.
I believe ConsoleStdin::read() has the same possibility, should I just do a separate commit in this pr?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sure, go ahead! :)

@GloriousAlpaca
GloriousAlpaca force-pushed the fix/console-stdin-poll-waker branch 2 times, most recently from f43b81b to aafb6a5 Compare October 29, 2026 09:13

@mkroening mkroening left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me, thanks! :)

@mkroening
mkroening enabled auto-merge October 29, 2026 09:14
@mkroening
mkroening disabled auto-merge October 29, 2026 09:31
@GloriousAlpaca
GloriousAlpaca force-pushed the fix/console-stdin-poll-waker branch from aafb6a5 to b2097d0 Compare October 29, 2026 09:42

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ConsoleStdin::poll()does not register a waker, causing select() to remain blocked

2 participants