cpu/native: use async read for stdio_read() #19002

benpicco · 2022-12-01T16:34:19Z

Contribution description

The real_read() function will block the thread but won't preempt it.
That means all other threads on the same (or higher) priority level are blocked as RIOT still considers the thread that called stdio_read() as running.

Use async_read/isrpipe to properly block the thread when reading from stdin.

Testing procedure

Run a 2nd thread with the same priority as the main thread:

--- a/tests/shell/main.c
+++ b/tests/shell/main.c
@@ -132,10 +132,28 @@ static int _xfa_test2(int argc, char **argv)
 SHELL_COMMAND(xfa_test2, "xfa test command 2", _xfa_test2);
 SHELL_COMMAND(xfa_test1, "xfa test command 1", _xfa_test1);
 
+#include "thread.h"
+#include "ztimer.h"
+
+static char _stack[1024];
+
+static void *_func(void *arg)
+{
+    while (1) {
+        ztimer_sleep(ZTIMER_MSEC, 1000);
+        puts(arg);
+    }
+
+    return NULL;
+}
+
 int main(void)
 {
     printf("test_shell.\n");
 
+    thread_create(_stack, sizeof(_stack), THREAD_PRIORITY_MAIN, THREAD_CREATE_STACKTEST,
+                  _func, "Hello shell", "periodic");
+
     /* define own shell commands */
     shell_run_once(shell_commands, line_buf, sizeof(line_buf));

On master the periodic "Hello shell" message is never printed.

Issues/PRs references

fixes #16834

riot-ci · 2022-12-02T05:46:33Z

Murdock results

✔️ PASSED

3ce2264 tests/pkg/lua_loader: disable test on native32

Success	Failures	Total	Runtime
10268	0	10269	13m:14s

Artifacts

Documentation preview

benpicco · 2022-12-02T15:42:50Z

@leandrolanzieri do you have an idea how to solve the Kconfig issue?

cpu/native/stdio_native/Kconfig

cpu/native/async_read.c

cpu/native/include/async_read.h

gschorcht

Looks good and works as expected.

gschorcht · 2022-12-20T18:29:29Z

Please squash.

benpicco · 2022-12-20T19:08:56Z

CI will still fail as some tests that use float will now preempt, which triggers #495

The real_read() function will block the thread but won't preempt it. That means all other thereads on the same (or higher) priority level are blocked as RIOT still consideres the thread that called stdio_read() as running. Use async_read/isrpipe to properly block the thread when reading from stdin.

It's no longer working on native, but native behaves more like a real board now.

Now that RIOT-OS#19002 has been merged, the test is also working on `native`.

mguetschow · 2025-08-06T15:36:19Z

This seems to break when more than STDIO_RX_BUFSIZE characters are input at once on the shell, with

RIOT/cpu/native/stdio_native/stdio_native.c

Lines 21 to 30 in db345a7

    
           static void _async_read_wrapper(int fd, void *arg) { 
        
               uint8_t buf[STDIO_RX_BUFSIZE]; 
        
               int res = real_read(fd, &buf, sizeof(buf)); 
        
               if (res > 0) { 
        
                   isrpipe_write(arg, buf, res); 
        
               } 
        
               native_async_read_continue(fd); 
        
           }

not checking the return value of isrpipe_write. This seems to produce undeterministically garbaged output for shell_readline. This is especially confusing to the user of the shell who configured the shell size to a larger value than the the default 64B.

Not sure what it actually should do with the return value, however :/

Any ideas how to fix that?

mguetschow · 2025-08-06T15:39:28Z

Oh, interesting:

RIOT/sys/include/shell.h

Lines 102 to 118 in e6a570a

    
           /** 
        
            * @brief Default shell buffer size (maximum line length shell can handle) 
        
            * 
        
            * @warning When terminals that buffer input and send the full command line in 
        
            *   one go are used on stdin implementations with fast bursts of data, 
        
            *   it may be necessary to increase the @ref STDIO_RX_BUFSIZE to make 
        
            *   practical use of this buffer, especially because the current mechanism of 
        
            *   passing stdin (`isrpipe_t stdin_isrpipe`) does not support backpressure 
        
            *   and overflows silently. As a consequence, commands through such terminals 
        
            *   appear to be truncated at @ref STDIO_RX_BUFSIZE bytes (defaulting to 64) 
        
            *   unless the command is sent in parts (on many terminals, by pressing Ctrl-D 
        
            *   half way through the command). 
        
            * 
        
            *   For example, this affects systems with direct USB stdio (@ref 
        
            *   usbus_cdc_acm_stdio) with the default terminal `pyterm`. 
        
            */ 
        
           #define SHELL_DEFAULT_BUFSIZE   (128)

github-actions bot added Area: cpu Area: CPU/MCU ports Platform: native Platform: This PR/issue effects the native platform labels Dec 1, 2022

benpicco requested review from miri64 and kaspar030 December 1, 2022 16:35

benpicco added the Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) label Dec 1, 2022

benpicco requested a review from kfessel December 1, 2022 17:04

benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 2, 2022

benpicco requested review from leandrolanzieri, jia200x and MrKevinWeiss as code owners December 2, 2022 14:42

github-actions bot added Area: Kconfig Area: Kconfig integration Area: tests Area: tests and testing framework labels Dec 2, 2022

benpicco mentioned this pull request Dec 2, 2022

tests/shell: add test for preemption #19005

Merged

benpicco requested a review from maribu December 2, 2022 15:22

benpicco force-pushed the cpu/native-read_preempt branch from 04cbcef to 9046e8a Compare December 2, 2022 15:31

leandrolanzieri reviewed Dec 2, 2022

View reviewed changes

cpu/native/stdio_native/Kconfig Outdated Show resolved Hide resolved

benpicco force-pushed the cpu/native-read_preempt branch from 9046e8a to bfdf538 Compare December 2, 2022 16:26

github-actions bot added Area: sys Area: System and removed Area: Kconfig Area: Kconfig integration labels Dec 2, 2022

benpicco force-pushed the cpu/native-read_preempt branch from bfdf538 to 42e8122 Compare December 2, 2022 16:34

github-actions bot added the Area: examples Area: Example Applications label Dec 2, 2022

gschorcht reviewed Dec 20, 2022

View reviewed changes

cpu/native/async_read.c Show resolved Hide resolved

gschorcht reviewed Dec 20, 2022

View reviewed changes

cpu/native/include/async_read.h Outdated Show resolved Hide resolved

gschorcht approved these changes Dec 20, 2022

View reviewed changes

benpicco force-pushed the cpu/native-read_preempt branch from ebac29d to ad755ef Compare December 20, 2022 19:07

benpicco mentioned this pull request Jan 5, 2023

native getchar is blocking RIOT #16834

Closed

github-actions bot removed the Area: USB Area: Universal Serial Bus label Feb 9, 2024

benpicco force-pushed the cpu/native-read_preempt branch from 17f8f9c to c65dd56 Compare February 9, 2024 18:14

github-actions bot added the Area: network Area: Networking label Feb 10, 2024

benpicco force-pushed the cpu/native-read_preempt branch from 41ce809 to bc3e7d5 Compare March 12, 2025 23:22

github-actions bot removed the Area: pkg Area: External package ports label Mar 12, 2025

benpicco added 4 commits March 13, 2025 00:23

cpu/native: bump ASYNC_READ_NUMOF

333e1ea

cpu/native: async_read: make sure not to close stdin on reboot

4232eb3

tests/shell: drop test that only ever worked on native

7c3637a

It's no longer working on native, but native behaves more like a real board now.

benpicco force-pushed the cpu/native-read_preempt branch from bc3e7d5 to 67ce013 Compare March 12, 2025 23:26

benpicco enabled auto-merge March 12, 2025 23:31

benpicco force-pushed the cpu/native-read_preempt branch from 67ce013 to 15add22 Compare March 12, 2025 23:47

cpu/native: port stdio to new interface

4889bb5

benpicco force-pushed the cpu/native-read_preempt branch from 15add22 to c5eb4ab Compare March 12, 2025 23:50

benpicco added this pull request to the merge queue Mar 13, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 13, 2025

benpicco added 2 commits March 13, 2025 09:58

sys/net/telnet: align API with stdio API

7a5cd93

tests/pkg/lua_loader: disable test on native32

3ce2264

benpicco force-pushed the cpu/native-read_preempt branch from 125b255 to 3ce2264 Compare March 13, 2025 08:58

benpicco added this pull request to the merge queue Mar 13, 2025

Merged via the queue into RIOT-OS:master with commit 5bcddc0 Mar 13, 2025
25 checks passed

benpicco deleted the cpu/native-read_preempt branch March 13, 2025 11:37

benpicco mentioned this pull request Mar 13, 2025

cpu/native: Gardening/QoL #21283

Merged

OlegHahm mentioned this pull request Mar 19, 2025

native: Exiting pyterm does not terminate the native process any more #21307

Closed

mguetschow added this to the Release 2025.04 milestone Apr 8, 2025

benpicco mentioned this pull request Apr 14, 2025

tests/shell: enable the test on native #21413

Merged

benpicco added a commit to benpicco/RIOT that referenced this pull request Apr 14, 2025

tests/shell: enable the test on native

f404519

Now that RIOT-OS#19002 has been merged, the test is also working on `native`.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

cpu/native: use async read for stdio_read() #19002

cpu/native: use async read for stdio_read() #19002

Uh oh!

benpicco commented Dec 1, 2022 •

edited

Loading

Uh oh!

riot-ci commented Dec 2, 2022 •

edited

Loading

Uh oh!

benpicco commented Dec 2, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gschorcht left a comment

Uh oh!

gschorcht commented Dec 20, 2022

Uh oh!

benpicco commented Dec 20, 2022

Uh oh!

Uh oh!

Uh oh!

mguetschow commented Aug 6, 2025

Uh oh!

mguetschow commented Aug 6, 2025

Uh oh!

Uh oh!

cpu/native: use async read for stdio_read() #19002

cpu/native: use async read for stdio_read() #19002

Uh oh!

Conversation

benpicco commented Dec 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

riot-ci commented Dec 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Murdock results

Artifacts

Uh oh!

benpicco commented Dec 2, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gschorcht left a comment

Choose a reason for hiding this comment

Uh oh!

gschorcht commented Dec 20, 2022

Uh oh!

benpicco commented Dec 20, 2022

Uh oh!

Uh oh!

Uh oh!

mguetschow commented Aug 6, 2025

Uh oh!

mguetschow commented Aug 6, 2025

Uh oh!

Uh oh!

benpicco commented Dec 1, 2022 •

edited

Loading

riot-ci commented Dec 2, 2022 •

edited

Loading