From c18ed8c4f9dd2d2fe4a15e30eeeacce7012f962a Mon Sep 17 00:00:00 2001 From: Kevin Ushey Date: Tue, 8 Sep 2026 10:36:46 -0700 Subject: [PATCH 1/2] Stop the Windows timer thread cooperatively --- NEWS.md | 4 ++ src/thread.c | 63 +++++++++++++++++++++-------- tests/testthat/test-timer.R | 81 +++++++++++++++++++++++++++++++++++++ 3 files changed, 131 insertions(+), 17 deletions(-) diff --git a/NEWS.md b/NEWS.md index bb4f73768..e32e467bb 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,9 @@ # cli (development version) +* On Windows, cli now wakes and joins its timer thread during cleanup, + instead of cancelling it asynchronously. Cleanup also works when + `PROCESSOR_ARCHITECTURE` is unset (#375, #494). + * `keypress()` improvements: - `timeout` argument to wait at most a given number of seconds for a key press. diff --git a/src/thread.c b/src/thread.c index 431af440a..796ff4e44 100644 --- a/src/thread.c +++ b/src/thread.c @@ -7,7 +7,10 @@ #endif #include -#ifndef _WIN32 +#ifdef _WIN32 +#include +#include +#else #include #endif @@ -20,14 +23,24 @@ double cli_speed_time = 1.0; volatile int cli__reset = 1; static int unloaded = 0; +#ifdef _WIN32 +static HANDLE tick_stop_event = NULL; +#endif + void* clic_thread_func(void *arg) { -#ifndef _WIN32 +#ifdef _WIN32 + DWORD timeout = cli__tick_ts.tv_sec * 1000 + cli__tick_ts.tv_nsec / 1000000; + /* Wake immediately on shutdown, even if the next tick is far away. */ + while (WaitForSingleObject(tick_stop_event, timeout) == WAIT_TIMEOUT) { + if (cli__reset) cli__timer_flag = 1; + } + return NULL; +#else sigset_t set; sigfillset(&set); int ret = pthread_sigmask(SIG_SETMASK, &set, NULL); /* We chicken out if the signals cannot be blocked. */ if (ret) return NULL; -#endif int old; pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, &old); @@ -37,6 +50,7 @@ void* clic_thread_func(void *arg) { nanosleep(&cli__tick_ts, NULL); if (cli__reset) cli__timer_flag = 1; } +#endif } int cli__start_thread(SEXP ticktime, SEXP speedtime) { @@ -51,16 +65,24 @@ int cli__start_thread(SEXP ticktime, SEXP speedtime) { cli__reset = 0; #else if (! getenv("CLI_NO_THREAD")) { +#ifdef _WIN32 + tick_stop_event = CreateEvent(NULL, TRUE, FALSE, NULL); + if (!tick_stop_event) return EAGAIN; +#endif ret = pthread_create( & tick_thread, /* attr = */ 0, clic_thread_func, /* arg = */ NULL ); - /* detaching makes it easier to clean up resources - * On Windows this causes issues and the thread cannot - * be cancelled, so we don't do it there. */ -#ifndef _WIN32 +#ifdef _WIN32 + if (ret) { + CloseHandle(tick_stop_event); + tick_stop_event = NULL; + tick_thread = 0; + } +#else + /* Windows keeps the thread joinable so cleanup can wait for it. */ if (!ret) pthread_detach(tick_thread); #endif } else { @@ -86,17 +108,23 @@ int cli__kill_thread(void) { int ret = 0; #ifdef _WIN32 + if (!tick_thread) return 0; - // On ARM64 builds of Windows (when running through x86 emulation), - // cancelling the running tick thread seems to cause issues during - // process shutdown. Avoid the issue by just neglecting to cancel - // the thread altogether. - const char* arch = getenv("PROCESSOR_ARCHITECTURE"); - if (!strcmp(arch, "ARM64")) { - return 0; + /* Do not asynchronously cancel a thread inside the Windows runtime. + * Let it return normally, and wait before releasing its resources. */ + if (!SetEvent(tick_stop_event)) { + ret = EINVAL; + } else { + ret = pthread_join(tick_thread, NULL); } - -#endif + if (ret) { + warning("Could not stop cli thread"); + return ret; + } + CloseHandle(tick_stop_event); + tick_stop_event = NULL; + tick_thread = 0; +#else /* This should not happen, but be extra careful */ if (tick_thread) { @@ -111,6 +139,7 @@ int cli__kill_thread(void) { return ret; // __NO_COVERAGE__ } } +#endif return ret; } @@ -122,7 +151,7 @@ int cli__kill_thread(void) { SEXP clic_stop_thread(void) { if (unloaded) return R_NilValue; int ret = 1; -#if defined(__clang__) && defined(__has_feature) +#if defined(__clang__) && defined(__has_feature) && !defined(_WIN32) # if __has_feature(address_sanitizer) /* clang in ASAN, do nothing */ # else diff --git a/tests/testthat/test-timer.R b/tests/testthat/test-timer.R index 02b965c78..a815067f3 100644 --- a/tests/testthat/test-timer.R +++ b/tests/testthat/test-timer.R @@ -15,3 +15,84 @@ test_that("cli_tick_set", { skip_on_cran() expect_silent(cli_tick_set()) }) + +test_that("Windows timer cleanup does not depend on the architecture env var", { + skip_if_not(.Platform$OS.type == "windows") + + for (no_thread in c(NA_character_, "true")) { + out <- callr::r( + function() { + loadNamespace("cli") + cli:::unload() + cli:::unload() + TRUE + }, + env = c(PROCESSOR_ARCHITECTURE = NA_character_, CLI_NO_THREAD = no_thread), + timeout = 10 + ) + expect_true(out) + } +}) + +test_that("Windows timer can restart and stop during a long wait", { + skip_if_not(.Platform$OS.type == "windows") + + out <- callr::r( + function() { + loadNamespace("cli") + for (i in seq_len(10)) { + cli:::cli_tick_set(60000L) + } + cli:::cli_tick_set(10L) + cli::cli_tick_reset() + deadline <- Sys.time() + 5 + while (!cli::`__cli_update_due` && Sys.time() < deadline) Sys.sleep(0.01) + ticking <- isTRUE(cli::`__cli_update_due`) + cli:::cli_tick_set(60000L) + Sys.setenv(CLI_NO_THREAD = "true") + cli:::cli_tick_set() + cli:::cli_tick_set() + cli:::unload() + ticking + }, + env = c(CLI_NO_THREAD = NA_character_, CLI_TICK_TIME = "60000", + CLI_SPEED_TIME = "1"), + timeout = 15 + ) + expect_true(out) +}) + +test_that("Windows sessions exit with a sleeping timer thread", { + skip_if_not(.Platform$OS.type == "windows") + + for (i in seq_len(3)) { + out <- callr::r( + function() { + loadNamespace("cli") + TRUE + }, + env = c(CLI_NO_THREAD = NA_character_, CLI_TICK_TIME = "60000", + CLI_SPEED_TIME = "1"), + timeout = 10 + ) + expect_true(out) + } +}) + +test_that("Windows timer finishes before its DLL is unloaded", { + skip_if_not(.Platform$OS.type == "windows") + + out <- callr::r( + function() { + for (i in seq_len(10)) { + loadNamespace("cli") + unloadNamespace("cli") + } + !"cli" %in% names(getLoadedDLLs()) + }, + env = c(CLI_NO_THREAD = NA_character_, CLI_TICK_TIME = "60000", + CLI_SPEED_TIME = "1"), + timeout = 15 + ) + expect_true(out) +}) From 66eb41ba4d843c2df0096d7bbbf30173a3ed9db9 Mon Sep 17 00:00:00 2001 From: Kevin Ushey Date: Tue, 8 Sep 2026 10:38:51 -0700 Subject: [PATCH 2/2] Format Windows timer regression tests --- tests/testthat/test-timer.R | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/tests/testthat/test-timer.R b/tests/testthat/test-timer.R index a815067f3..ac02fdd56 100644 --- a/tests/testthat/test-timer.R +++ b/tests/testthat/test-timer.R @@ -27,7 +27,10 @@ test_that("Windows timer cleanup does not depend on the architecture env var", { cli:::unload() TRUE }, - env = c(PROCESSOR_ARCHITECTURE = NA_character_, CLI_NO_THREAD = no_thread), + env = c( + PROCESSOR_ARCHITECTURE = NA_character_, + CLI_NO_THREAD = no_thread + ), timeout = 10 ) expect_true(out) @@ -46,7 +49,9 @@ test_that("Windows timer can restart and stop during a long wait", { cli:::cli_tick_set(10L) cli::cli_tick_reset() deadline <- Sys.time() + 5 - while (!cli::`__cli_update_due` && Sys.time() < deadline) Sys.sleep(0.01) + while (!cli::`__cli_update_due` && Sys.time() < deadline) { + Sys.sleep(0.01) + } ticking <- isTRUE(cli::`__cli_update_due`) cli:::cli_tick_set(60000L) Sys.setenv(CLI_NO_THREAD = "true") @@ -55,8 +60,11 @@ test_that("Windows timer can restart and stop during a long wait", { cli:::unload() ticking }, - env = c(CLI_NO_THREAD = NA_character_, CLI_TICK_TIME = "60000", - CLI_SPEED_TIME = "1"), + env = c( + CLI_NO_THREAD = NA_character_, + CLI_TICK_TIME = "60000", + CLI_SPEED_TIME = "1" + ), timeout = 15 ) expect_true(out) @@ -71,8 +79,11 @@ test_that("Windows sessions exit with a sleeping timer thread", { loadNamespace("cli") TRUE }, - env = c(CLI_NO_THREAD = NA_character_, CLI_TICK_TIME = "60000", - CLI_SPEED_TIME = "1"), + env = c( + CLI_NO_THREAD = NA_character_, + CLI_TICK_TIME = "60000", + CLI_SPEED_TIME = "1" + ), timeout = 10 ) expect_true(out) @@ -90,8 +101,11 @@ test_that("Windows timer finishes before its DLL is unloaded", { } !"cli" %in% names(getLoadedDLLs()) }, - env = c(CLI_NO_THREAD = NA_character_, CLI_TICK_TIME = "60000", - CLI_SPEED_TIME = "1"), + env = c( + CLI_NO_THREAD = NA_character_, + CLI_TICK_TIME = "60000", + CLI_SPEED_TIME = "1" + ), timeout = 15 ) expect_true(out)