Merge pull request #2282 from knewstimek/pr/windows-worker-recovery-fix

fix(windows): preserve worker recovery errors

Author: DeusData

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
 

src/mcp/index_supervisor.c

@@ -13,6 +13,7 @@
 #include "ui/http_server.h"      /* cbm_http_server_resolve_binary_path */
 
 #include <limits.h>
+#include <errno.h>
 #include <stdio.h>
 #include <stdint.h>
 #include <stdlib.h>
@@ -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);

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 */
         }

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);