From 1d2bbc425988eb78fcd966dc8c9cfbdb814f0685 Mon Sep 17 00:00:00 2001 From: News Date: Tue, 22 Sep 2026 22:33:51 +0900 Subject: [PATCH] fix(windows): preserve worker recovery errors Signed-off-by: News --- src/foundation/compat.c | 52 +++++++++++++++++++++++++------------- src/mcp/index_supervisor.c | 6 +++++ src/mcp/mcp.c | 5 +++- tests/test_platform.c | 41 +++++++++++++++++++++++++++--- 4 files changed, 82 insertions(+), 22 deletions(-) diff --git a/src/foundation/compat.c b/src/foundation/compat.c index 94e7020bd7..fb3caca871 100644 --- a/src/foundation/compat.c +++ b/src/foundation/compat.c @@ -269,36 +269,54 @@ int cbm_mkstemp(char *tmpl) { errno = ENAMETOOLONG; return CBM_NOT_FOUND; } - /* Wide-API expansion and open: worker staging files land inside - * CBM_CACHE_DIR, which users may place at non-ASCII paths; the ANSI CRT - * (_mktemp/_open) mangles those bytes in the local codepage. */ + /* Keep the six-character mkstemp contract, but do not use _wmktemp: + * that CRT helper has a tiny name space on Windows and retained worker + * logs can exhaust it during recovery. The exclusive open closes races + * with other processes; collisions simply draw another random name. */ wchar_t *wide_template = cbm_utf8_to_wide(buf); - if (!wide_template || !_wmktemp(wide_template)) { - free(wide_template); + if (!wide_template) { + errno = EINVAL; return CBM_NOT_FOUND; } - char *expanded_for_open = cbm_wide_to_utf8(wide_template); - wchar_t *wide_open = expanded_for_open ? cbm_path_to_wide(expanded_for_open) : NULL; - free(expanded_for_open); - if (!wide_open) { + size_t wide_len = wcslen(wide_template); + if (wide_len < 6 || wcscmp(wide_template + wide_len - 6, L"XXXXXX") != 0) { free(wide_template); + errno = EINVAL; return CBM_NOT_FOUND; } - int fd = _wopen(wide_open, _O_CREAT | _O_EXCL | _O_RDWR | _O_BINARY, _S_IREAD | _S_IWRITE); - free(wide_open); - if (fd >= 0) { + static const wchar_t hex[] = L"0123456789abcdef"; + for (int attempt = 0; attempt < 128; attempt++) { + unsigned int random_bits = 0; + if (!cbm_secure_random(&random_bits, sizeof(random_bits))) { + errno = EIO; + break; + } + for (int digit = 0; digit < 6; digit++) { + wide_template[wide_len - 6 + digit] = hex[(random_bits >> (digit * 4)) & 0xf]; + } char *expanded = cbm_wide_to_utf8(wide_template); - if (!expanded || strlen(expanded) >= sizeof(buf)) { + wchar_t *wide_open = expanded ? cbm_path_to_wide(expanded) : NULL; + if (!expanded || !wide_open || strlen(expanded) >= sizeof(buf)) { + free(expanded); + free(wide_open); + errno = ENAMETOOLONG; + break; + } + int fd = _wopen(wide_open, _O_CREAT | _O_EXCL | _O_RDWR | _O_BINARY, _S_IREAD | _S_IWRITE); + free(wide_open); + if (fd >= 0) { + strcpy(tmpl, expanded); free(expanded); free(wide_template); - (void)_close(fd); - return CBM_NOT_FOUND; + return fd; } - strcpy(tmpl, expanded); free(expanded); + if (errno != EEXIST) { + break; + } } free(wide_template); - return fd; + return CBM_NOT_FOUND; } #endif diff --git a/src/mcp/index_supervisor.c b/src/mcp/index_supervisor.c index 4cd69f6426..5e15a7eed0 100644 --- a/src/mcp/index_supervisor.c +++ b/src/mcp/index_supervisor.c @@ -13,6 +13,7 @@ #include "ui/http_server.h" /* cbm_http_server_resolve_binary_path */ #include +#include #include #include #include @@ -705,6 +706,11 @@ int cbm_index_worker_start_with_log(const char *args_json, size_t memory_budget_ worker_result_init(&handle->result); if (!worker_unique_file(handle->response_path, sizeof(handle->response_path), "response") || !worker_unique_file(handle->log_path, sizeof(handle->log_path), "log")) { + int saved_errno = errno; + char error_text[CBM_SZ_32]; + (void)snprintf(error_text, sizeof(error_text), "%d", saved_errno); + cbm_log_error("index.supervisor.artifact_create_failed", "artifact", + handle->response_path[0] ? "log" : "response", "errno", error_text); (void)cbm_unlink(handle->response_path); (void)cbm_unlink(handle->log_path); free(handle); diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 75ff8e396c..d6c7bd4686 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -10808,7 +10808,10 @@ static char *index_run_supervised(cbm_mcp_server_t *srv, const char *args) { cbm_mcp_supervised_result_disposition_t recovery_disposition = cbm_mcp_supervised_result_disposition(rc2, &wr2); if (recovery_disposition == CBM_MCP_SUPERVISED_RESULT_FALLBACK) { - last_outcome = wr2.outcome; + /* Keep the original worker failure if recovery setup fails. */ + cbm_log_error("index.supervisor.recovery_start_failed", "original_outcome", + cbm_proc_outcome_str(last_outcome), "recovery_outcome", + cbm_proc_outcome_str(wr2.outcome)); cbm_index_worker_result_free(&wr2); break; /* spawn failed mid-recovery — give up */ } diff --git a/tests/test_platform.c b/tests/test_platform.c index 5995a4b017..16cc3b5d31 100644 --- a/tests/test_platform.c +++ b/tests/test_platform.c @@ -46,8 +46,7 @@ TEST(platform_file_apis_survive_max_path_overflow) { ASSERT_NOT_NULL(cbm_mkdtemp(base)); enum { LONG_SEGMENTS = 5 }; - static const char segment[] = - "segment-abcdefghijklmnopqrstuvwxyz0123456789-abcdefghijklmnop"; + static const char segment[] = "segment-abcdefghijklmnopqrstuvwxyz0123456789-abcdefghijklmnop"; char deep[CBM_SZ_1K]; written = snprintf(deep, sizeof(deep), "%s", base); ASSERT_TRUE(written > 0 && written < (int)sizeof(deep)); @@ -475,6 +474,28 @@ TEST(platform_mkstemp_and_mkdtemp_survive_non_ascii_directory) { PASS(); } +#ifdef _WIN32 +TEST(platform_mkstemp_retained_files_exceed_crt_namespace) { + char base[CBM_SZ_256] = "/tmp/cbm-many-temp-XXXXXX"; + ASSERT_NOT_NULL(cbm_mkdtemp(base)); + char paths[64][CBM_SZ_512] = {{0}}; + int created = 0; + for (; created < 64; created++) { + int written = + snprintf(paths[created], sizeof(paths[created]), "%s/.worker-log-XXXXXX", base); + ASSERT_TRUE(written > 0 && written < (int)sizeof(paths[created])); + int descriptor = cbm_mkstemp(paths[created]); + ASSERT_TRUE(descriptor >= 0); + ASSERT_EQ(_close(descriptor), 0); + } + for (int index = 0; index < created; index++) { + ASSERT_EQ(cbm_unlink(paths[index]), 0); + } + ASSERT_EQ(cbm_rmdir(base), 0); + PASS(); +} +#endif + typedef struct { atomic_int *ready; atomic_bool *go; @@ -832,8 +853,17 @@ TEST(platform_env_long_refuses_what_it_cannot_read) { /* Every one of these used to answer 0 through atol. */ const char *unreadable[] = { - "abc", "30s", " 30", "30 ", "", "1e3", - "0x10", "+ 30", "--3", "3.5", "99999999999999999999999999", + "abc", + "30s", + " 30", + "30 ", + "", + "1e3", + "0x10", + "+ 30", + "--3", + "3.5", + "99999999999999999999999999", }; for (size_t i = 0; i < sizeof(unreadable) / sizeof(unreadable[0]); i++) { ASSERT_EQ(cbm_setenv(name, unreadable[i], 1), 0); @@ -1135,6 +1165,9 @@ SUITE(platform) { RUN_TEST(platform_mkdir_p_follow_owned_is_per_call_site); RUN_TEST(platform_mkdir_p_resolves_link_text_from_the_link_directory); RUN_TEST(platform_mkstemp_and_mkdtemp_survive_non_ascii_directory); +#ifdef _WIN32 + RUN_TEST(platform_mkstemp_retained_files_exceed_crt_namespace); +#endif RUN_TEST(platform_mkdtemp_is_thread_safe); RUN_TEST(platform_counter_scaling_avoids_intermediate_overflow); RUN_TEST(platform_counter_scaling_preserves_monotonic_deadlines);