From ad94694fbd8aa35a72ea53d03420d56281a69f04 Mon Sep 17 00:00:00 2001 From: WillyJL Date: Wed, 24 Sep 2025 11:19:18 +0200 Subject: [PATCH] CLI: Fix long delay with quick connect/disconnect (#4251) * CLI: Fix long delay with quick connect/disconnect noticeable with qflipper, for some reason it sets DTR on/off/on again so flipper starts CLI, stops it, then starts again but cli shell is trying to print motd, and pipe is already broken so each character of motd times out for 100ms changing pipe timeout to 10ms works too, but is just a workaround it didnt always happen, i think that variable time of loading ext cmds made it so on ofw it usually manages to print motd before pipe is broken on cfw with more ext commands it always hung, on ofw only sometimes important part is bailing early in cli shell in cli vcp i made it cleanup cli shell on disconnect so it doesnt stay around after disconnecting usb, might free a little ram maybe * cli_shell: possibly more robust fix? * Fix use after free crash * cli_shell: waste even less time Co-Authored-By: WillyJL --------- Co-authored-by: Anna Antonenko Co-authored-by: hedger --- applications/services/cli/cli_vcp.c | 13 +++++-------- lib/toolbox/cli/shell/cli_shell.c | 16 +++++++++++----- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/applications/services/cli/cli_vcp.c b/applications/services/cli/cli_vcp.c index 1f9c77b08..9db1bec43 100644 --- a/applications/services/cli/cli_vcp.c +++ b/applications/services/cli/cli_vcp.c @@ -218,6 +218,11 @@ static void cli_vcp_internal_event_happened(FuriEventLoopObject* object, void* c pipe_detach_from_event_loop(cli_vcp->own_pipe); pipe_free(cli_vcp->own_pipe); cli_vcp->own_pipe = NULL; + + // wait for shell to stop + cli_shell_join(cli_vcp->shell); + cli_shell_free(cli_vcp->shell); + pipe_free(cli_vcp->shell_pipe); break; } @@ -226,14 +231,6 @@ static void cli_vcp_internal_event_happened(FuriEventLoopObject* object, void* c FURI_LOG_D(TAG, "Connected"); cli_vcp->is_connected = true; - // wait for previous shell to stop - furi_check(!cli_vcp->own_pipe); - if(cli_vcp->shell) { - cli_shell_join(cli_vcp->shell); - cli_shell_free(cli_vcp->shell); - pipe_free(cli_vcp->shell_pipe); - } - // start shell thread PipeSideBundle bundle = pipe_alloc(VCP_BUF_SIZE, 1); cli_vcp->own_pipe = bundle.alices_side; diff --git a/lib/toolbox/cli/shell/cli_shell.c b/lib/toolbox/cli/shell/cli_shell.c index 7a4c7ec1f..b2648d127 100644 --- a/lib/toolbox/cli/shell/cli_shell.c +++ b/lib/toolbox/cli/shell/cli_shell.c @@ -16,7 +16,8 @@ #define TAG "CliShell" -#define ANSI_TIMEOUT_MS 10 +#define ANSI_TIMEOUT_MS 10 +#define TRANSIENT_SESSION_WINDOW_MS 100 typedef enum { CliShellComponentCompletions, @@ -415,10 +416,15 @@ static void cli_shell_deinit(CliShell* shell) { static int32_t cli_shell_thread(void* context) { CliShell* shell = context; - // Sometimes, the other side closes the pipe even before our thread is started. Although the - // rest of the code will eventually find this out if this check is removed, there's no point in - // wasting time. - if(pipe_state(shell->pipe) == PipeStateBroken) return 0; + // Sometimes, the other side (e.g. qFlipper) closes the pipe even before our thread is started. + // Although the rest of the code will eventually find this out if this check is removed, + // there's no point in wasting time. This gives qFlipper a chance to quickly close and re-open + // the session. + const size_t delay_step = 10; + for(size_t i = 0; i < TRANSIENT_SESSION_WINDOW_MS / delay_step; i++) { + furi_delay_ms(delay_step); + if(pipe_state(shell->pipe) == PipeStateBroken) return 0; + } cli_shell_init(shell); FURI_LOG_D(TAG, "Started");