From 19052674bdf64ff48c6b88de30b385c42b343fca Mon Sep 17 00:00:00 2001
From: Rongsong Shen <rshen@shenrs.eu>
Date: Sat, 10 Oct 2026 14:41:02 +0000
Subject: [PATCH] fix(autoscaler): run provider scripts with posix_spawn, not
 fork (bug-1969)

The scaler is multi-threaded (monitor and demand threads); invoke() forked
and, in the child, logged, snprintf'd and setenv()'d before execl(), which
can deadlock on a lock another thread held at fork time and block the
caller in waitpid() forever.

run_script() now posix_spawn()s `sh <script> -f <in.json>`: stdout and
stderr go to the output file through file actions, the provider variables
(PRO_CONF_DIR, PRO_DATA_DIR, PRO_LOG_DIR, PROVIDER_NAME) are passed as the
child's envp (provider_env()), the child starts with an empty signal mask
and default dispositions, and waitpid() retries on EINTR.

The scaler unit suite (scaler/testsuite) was never built; it is now the
meson test "autoscaler test", linked with -Wl,--wrap=fork so it counts
fork() calls during a provider call.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
---
 .kiro/steering/tech.md                        |   2 +-
 scripts/check-no-fork.py                      |   3 +-
 src/autoscaler/meson.build                    |  16 ++
 src/autoscaler/scaler/plug_mgr.c              | 189 +++++++++++++-----
 .../scripts/getAvailableTemplates.sh          |  15 ++
 src/autoscaler/scaler/testsuite/test.c        |   2 +-
 .../scaler/testsuite/test_plug_mgr.c          |  53 ++++-
 .../scaler/testsuite/test_scale_misc.c        |   2 +-
 8 files changed, 222 insertions(+), 60 deletions(-)
 create mode 100644 src/autoscaler/scaler/testsuite/envscripts/scripts/getAvailableTemplates.sh

diff --git a/.kiro/steering/tech.md b/.kiro/steering/tech.md
index 64ba8f69..8e792f25 100644
--- a/.kiro/steering/tech.md
+++ b/.kiro/steering/tech.md
@@ -91,7 +91,7 @@ Early return on every error. Log with zap fields before returning.
 - **Threaded processes**: the controller, the scheduler, the `xcecd` daemon (`xcecd`, `xcecd_virtual`: thread pool, network, resource-sync and connector threads), and any process that starts libxce's delaymgr/netio/vtaskmgr threads (`xce_ctx_start()`).
 - **How to start a child instead**: `posix_spawn()`/`posix_spawnp()` (glibc runs it with `CLONE_VM|CLONE_VFORK`, no copied user code), or a systemd unit over D-Bus as `xcecd` does for `xcecd_worker`. Setup that `posix_spawn()` cannot do (setuid, rlimits, cgroup join) belongs in a single-threaded process such as `xcecd_worker`, never in the threaded daemon.
 - **Allowed**: `exec*()` without a fork (process replacement, e.g. the controller's HA self-restart); and a fork-family call in a process proven single-threaded, marked on the call line or the line above with `/* xce-fork-ok: <why> */`. `xcecd_worker` (the per-job process) is single-threaded: its call graph from `main()`, callbacks included, reaches no `pthread_create()` and no libuv thread-pool call (`uv_queue_work`, `uv_fs_*`, `uv_getaddrinfo`); re-prove it before adding a thread there.
-- **Scope**: C code under `src/controller`, `src/scheduler`, `src/lib`, `src/worker`. Excluded (decision 2026-10-10): test code (`testsuite/`, `test*` files; forking to isolate a test case is fine), the harness (`src/harness`, `xce_test`), the autoscaler (not finalized; its `plug_mgr.c` fork is tracked as a bug and must be fixed before it ships), and the Go frontend (`os/exec` is safe).
+- **Scope**: C code under `src/controller`, `src/scheduler`, `src/lib`, `src/worker`. Excluded (decision 2026-10-10): test code (`testsuite/`, `test*` files; forking to isolate a test case is fine), the harness (`src/harness`, `xce_test`), the autoscaler (not finalized; its provider calls use `posix_spawn()` since bug-1969, and its suite counts `fork()` calls through `-Wl,--wrap=fork`), and the Go frontend (`os/exec` is safe).
 - **Enforcement**: `scripts/check-no-fork.py` runs as the meson test `no-fork rule` (suites lib, controller, scheduler, worker, so `/xce-build` runs it) and in the pre-commit hook on staged C files.
 
 ### Auth and RBAC
diff --git a/scripts/check-no-fork.py b/scripts/check-no-fork.py
index 09bfcf7e..2a69c433 100644
--- a/scripts/check-no-fork.py
+++ b/scripts/check-no-fork.py
@@ -9,7 +9,8 @@ started with posix_spawn(), or by systemd over D-Bus.
 
 Scanned: C sources of the components the rule covers (COVERED below), minus
 test code. Excluded by decision (2026-10-10): tests, the harness (xce_test),
-the autoscaler (not finalized; tracked as a bug) and the Go frontend.
+the autoscaler (not finalized; provider calls use posix_spawn since bug-1969)
+and the Go frontend.
 
 A call is allowed only in a process proven single-threaded, marked on the call
 line or the line above with:  /* xce-fork-ok: <why it is single-threaded> */
diff --git a/src/autoscaler/meson.build b/src/autoscaler/meson.build
index d47d7349..d7b2e8c1 100644
--- a/src/autoscaler/meson.build
+++ b/src/autoscaler/meson.build
@@ -24,3 +24,19 @@ libscaler = static_library('scaler',
                            install: true)
 
 subdir('providers')
+
+# Scaler unit tests: plug_mgr drives the fake provider scripts in
+# scaler/testsuite/scripts (scriptPath "./testsuite"), so they run from
+# scaler/. --wrap=fork: the no-fork rule (tech.md "Process Creation",
+# bug-1969) -- the tests count fork() calls made while invoking a provider.
+scaler_test = executable('scaler_test',
+                         ['scaler/testsuite/test.c',
+                          'scaler/testsuite/test_plug_mgr.c',
+                          'scaler/testsuite/test_scale_misc.c'],
+                         include_directories: [incdir],
+                         dependencies: [libscaler_deps, yaml_deps, unity_dep],
+                         link_with: libscaler,
+                         link_args: ['-Wl,--wrap=fork'])
+test('autoscaler test', scaler_test,
+     workdir: meson.current_source_dir() / 'scaler',
+     suite: 'autoscaler')
diff --git a/src/autoscaler/scaler/plug_mgr.c b/src/autoscaler/scaler/plug_mgr.c
index 98e72297..c47c6bde 100644
--- a/src/autoscaler/scaler/plug_mgr.c
+++ b/src/autoscaler/scaler/plug_mgr.c
@@ -6,13 +6,19 @@
 #include "scaler_obj.h"
 #include "utils.h"
 #include <cjson/cJSON.h>
+#include <errno.h>
 #include <fcntl.h>
+#include <signal.h>
+#include <spawn.h>
 #include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
 #include <sys/wait.h>
 #include <unistd.h>
 
+extern char** environ;
+
 static cJSON* invoke(scaler_provider_t* prov, INTERFACE_ACTION ACTION,
                      cJSON* in_json);
 
@@ -379,18 +385,124 @@ int get_return_request(char* prov_name, instance_id_list_t* inst_ids)
     return 0;
 }
 
-// PRO_CONF_DIR
-// PRO_DATA_DIR
-// PRO_LOG_DIR
-// PROVIDER_NAME default to "baidu" if not set
-// TODO zcg need to set PRO_LOG_DIR
-static int export_env(scaler_provider_t* prov)
+static void free_env(char** envp)
 {
-    setenv("PRO_CONF_DIR", prov->confPath, 1);
-    setenv("PRO_DATA_DIR", "/tmp", 1);
-    setenv("PRO_LOG_DIR", "/tmp", 1);
-    setenv("PROVIDER_NAME", prov->name, 1);
-    return 0;
+    if (!envp) {
+        return;
+    }
+    for (char** e = envp; *e; e++) {
+        free(*e);
+    }
+    free(envp);
+}
+
+/*
+ * provider_env - the environment of a provider script: the scaler's own
+ * environment with PRO_CONF_DIR, PRO_DATA_DIR, PRO_LOG_DIR and
+ * PROVIDER_NAME set for this provider. Built for posix_spawn() so that
+ * neither the scaler (multi-threaded: setenv() is not thread-safe) nor a
+ * forked child (bug-1969) has to setenv(). Returns NULL on allocation
+ * failure; free with free_env().
+ */
+// TODO zcg need to set PRO_LOG_DIR
+static char** provider_env(scaler_provider_t* prov)
+{
+    const char* names[]  = { "PRO_CONF_DIR", "PRO_DATA_DIR", "PRO_LOG_DIR",
+                             "PROVIDER_NAME" };
+    const char* values[] = { prov->confPath, "/tmp", "/tmp", prov->name };
+    size_t      nset     = sizeof(names) / sizeof(names[0]);
+    size_t      nenv     = 0;
+    size_t      k        = 0;
+    char**      envp;
+
+    while (environ && environ[nenv]) {
+        nenv++;
+    }
+    envp = calloc(nenv + nset + 1, sizeof(char*));
+    if (!envp) {
+        return NULL;
+    }
+    for (size_t i = 0; i < nenv; i++) {
+        bool replaced = false;
+        for (size_t j = 0; j < nset && !replaced; j++) {
+            size_t len = strlen(names[j]);
+            replaced   = strncmp(environ[i], names[j], len) == 0
+                       && environ[i][len] == '=';
+        }
+        if (!replaced && !(envp[k++] = strdup(environ[i]))) {
+            free_env(envp);
+            return NULL;
+        }
+    }
+    for (size_t j = 0; j < nset; j++) {
+        if (asprintf(&envp[k++], "%s=%s", names[j], values[j] ? values[j] : "")
+            < 0) {
+            envp[k - 1] = NULL;
+            free_env(envp);
+            return NULL;
+        }
+    }
+    return envp;
+}
+
+/*
+ * run_script - run `sh <cmd> -f <in_file>` with stdout and stderr in
+ * out_file and wait for it. posix_spawn(), not fork(): the scaler is
+ * multi-threaded, and a forked child that logged, allocated or setenv()'d
+ * before exec could deadlock on a lock another thread held (bug-1969,
+ * tech.md "Process Creation"). The child starts with an empty signal mask
+ * and default signal dispositions. Returns the wait status, or -errno.
+ */
+static int run_script(scaler_provider_t* prov, const char* cmd,
+                      const char* in_file, const char* out_file)
+{
+    posix_spawn_file_actions_t fa;
+    posix_spawnattr_t          attr;
+    sigset_t                   none, all;
+    char*  argv[] = { "sh", (char*)cmd, "-f", (char*)in_file, NULL };
+    char** envp   = provider_env(prov);
+    pid_t  pid;
+    int    status = 0;
+    int    rc;
+
+    if (!envp) {
+        return -ENOMEM;
+    }
+    sigemptyset(&none);
+    sigfillset(&all);
+    posix_spawn_file_actions_init(&fa);
+    posix_spawnattr_init(&attr);
+    rc = posix_spawn_file_actions_addopen(&fa, STDOUT_FILENO, out_file,
+                                          O_WRONLY | O_CREAT | O_TRUNC, 0644);
+    if (rc == 0) {
+        rc = posix_spawn_file_actions_adddup2(&fa, STDOUT_FILENO,
+                                              STDERR_FILENO);
+    }
+    if (rc == 0) {
+        rc = posix_spawnattr_setflags(
+            &attr, POSIX_SPAWN_SETSIGMASK | POSIX_SPAWN_SETSIGDEF);
+    }
+    if (rc == 0) {
+        rc = posix_spawnattr_setsigmask(&attr, &none);
+    }
+    if (rc == 0) {
+        rc = posix_spawnattr_setsigdefault(&attr, &all);
+    }
+    if (rc == 0) {
+        rc = posix_spawn(&pid, "/bin/sh", &fa, &attr, argv, envp);
+    }
+    posix_spawnattr_destroy(&attr);
+    posix_spawn_file_actions_destroy(&fa);
+    free_env(envp);
+    if (rc != 0) {
+        return -rc;
+    }
+    while (waitpid(pid, &status, 0) < 0) {
+        if (errno != EINTR) {
+            return -errno;
+        }
+    }
+    return status;
 }
 
 static int write_input_json(const char* filename, cJSON* json)
@@ -422,9 +534,9 @@ static cJSON* invoke(scaler_provider_t* prov, INTERFACE_ACTION ACTION,
 {
     char   in_file[64]  = "/tmp/sc_in_XXXXXX";
     char   out_file[64] = "/tmp/sc_out_XXXXXX";
+    char   cmd[XCE_MAXPATH_LEN];
     cJSON* out_json     = NULL;
-    int    ret;
-    pid_t  pid;
+    int    status;
 
     if (!prov || !in_json) {
         ERROR("Invalid input JSON");
@@ -437,50 +549,17 @@ static cJSON* invoke(scaler_provider_t* prov, INTERFACE_ACTION ACTION,
     }
     write_input_json(in_file, in_json);
 
-    // Fork and exec the cmd to run the provider script
-    pid = fork();
-    if (pid == -1) {
-        ERROR("fork() failed");
+    // Run the provider script and wait for it
+    my_snprintf(cmd, sizeof(cmd), "%s/scripts/%s", prov->scriptPath,
+                prov->scripts[ACTION]);
+    status = run_script(prov, cmd, in_file, out_file);
+    if (status < 0) {
+        ERROR("Failed to run %s: %s", cmd, strerror(-status));
         goto _OUT_;
-    } else if (pid == 0) {
-        char cmd[XCE_MAXPATH_LEN];
-        int  fd;
-        // Open output file (overwrite mode)
-        fd = open(out_file, O_WRONLY | O_CREAT | O_TRUNC, 0644);
-        if (fd == -1) {
-            ERROR("Create %s failed", out_file);
-            exit(EXIT_FAILURE);
-        }
-        // Build command to call the script
-        my_snprintf(cmd, sizeof(cmd), "%s/scripts/%s", prov->scriptPath,
-                    prov->scripts[ACTION]);
-
-        // Redirect stdout and stderr to file
-        dup2(fd, STDOUT_FILENO);
-        dup2(fd, STDERR_FILENO);
-        close(fd);
-
-        // export environment variables
-        export_env(prov);
-        // Execute script
-        ret = execl("/bin/sh", "sh", cmd, "-f", in_file, (char*)NULL);
-
-        if (ret == -1) {
-            ERROR("execl() %s failed", cmd);
-            exit(EXIT_FAILURE); // Exit child process on failure
-        }
-
-        // If exec succeeds, it won't return here
-        exit(EXIT_SUCCESS);
+    } else if (WIFEXITED(status)) {
+        TRACE("Script exited with status: %d", WEXITSTATUS(status));
     } else {
-        int status;
-        waitpid(pid, &status, 0);
-
-        if (WIFEXITED(status)) {
-            TRACE("Script exited with status: %d", WEXITSTATUS(status));
-        } else {
-            INFO("Script terminated abnormally");
-        }
+        INFO("Script terminated abnormally");
     }
 
     read_json_file(out_file, &out_json);
diff --git a/src/autoscaler/scaler/testsuite/envscripts/scripts/getAvailableTemplates.sh b/src/autoscaler/scaler/testsuite/envscripts/scripts/getAvailableTemplates.sh
new file mode 100644
index 00000000..52daff4d
--- /dev/null
+++ b/src/autoscaler/scaler/testsuite/envscripts/scripts/getAvailableTemplates.sh
@@ -0,0 +1,15 @@
+#!/bin/sh
+# bug-1969: reports the provider environment the scaler gave this script.
+cat <<JSON
+{
+    "templates":[
+        {
+            "templateId":"${PROVIDER_NAME}:${PRO_CONF_DIR}:${PRO_DATA_DIR}",
+            "maxNumber":1,
+            "attributes":{
+                "ncpus":["Numeric","1"]
+            }
+        }
+    ]
+}
+JSON
diff --git a/src/autoscaler/scaler/testsuite/test.c b/src/autoscaler/scaler/testsuite/test.c
index 95a4f2d1..eb62b2a6 100644
--- a/src/autoscaler/scaler/testsuite/test.c
+++ b/src/autoscaler/scaler/testsuite/test.c
@@ -2,7 +2,7 @@
 #include <stdlib.h>
 #include <string.h>
 
-#include "unity.h"
+#include <unity/unity.h>
 
 extern void test_plug_mgr(void);
 extern void test_scale_misc(void);
diff --git a/src/autoscaler/scaler/testsuite/test_plug_mgr.c b/src/autoscaler/scaler/testsuite/test_plug_mgr.c
index f5f41231..fd1beb5b 100644
--- a/src/autoscaler/scaler/testsuite/test_plug_mgr.c
+++ b/src/autoscaler/scaler/testsuite/test_plug_mgr.c
@@ -1,5 +1,6 @@
 #include <stdlib.h>
 #include <string.h>
+#include <unistd.h>
 
 #include "obj_mgr.h"
 #include "plug_mgr.h"
@@ -7,7 +8,7 @@
 #include "scaler.h"
 #include "scaler_misc.h"
 #include "scaler_obj.h"
-#include "unity.h"
+#include <unity/unity.h>
 
 #define TEST_PROVIDER           "test-provider"
 #define TEST_PROVIDER_TYPE      "testProv"
@@ -18,8 +19,22 @@
 #define TEST_HOSTNAME           "test-hostname-1"
 #define TEST_PRIVATE_IP         "10.100.2.118"
 
+#define ENV_PROVIDER "env-provider"
+
 autoscaler_context_t g_scaler_ctx;
 
+/* bug-1969: linked with -Wl,--wrap=fork (meson.build), so every fork() made
+ * by the scaler lands here. The scaler is multi-threaded, so a provider call
+ * must start its script with posix_spawn(), never fork() (tech.md "Process
+ * Creation"). */
+pid_t        __real_fork(void);
+static int   fork_calls;
+pid_t        __wrap_fork(void)
+{
+    fork_calls++;
+    return __real_fork();
+}
+
 extern const char* PROV_SCRIPTS[];
 
 static void global_init(void)
@@ -44,6 +59,15 @@ static void global_init(void)
         = strdup(PROV_SCRIPTS[GET_RETURN_REQUEST]);
     prov->scripts[RETURN_MACHINES] = strdup(PROV_SCRIPTS[RETURN_MACHINES]);
     provider_add(&g_scaler_ctx.providers, prov);
+
+    /* a provider whose only script echoes its environment (bug-1969) */
+    prov = (scaler_provider_t*)malloc(sizeof(scaler_provider_t));
+    provider_init(prov, ENV_PROVIDER, TEST_PROVIDER_TYPE);
+    prov->confPath   = strdup("/etc/xce-env-conf");
+    prov->scriptPath = strdup("./testsuite/envscripts");
+    prov->scripts[GET_AVAILABLE_TEMPLATES]
+        = strdup(PROV_SCRIPTS[GET_AVAILABLE_TEMPLATES]);
+    provider_add(&g_scaler_ctx.providers, prov);
 }
 
 static void global_cleanup(void)
@@ -156,6 +180,31 @@ static void test_get_return_request(void)
     TEST_ASSERT_EQUAL(ALLOC_STATUS_RECLAIMED, inst->alloc_status);
 }
 
+/* bug-1969: a provider call runs its script without fork(). */
+static void test_provider_call_does_not_fork(void)
+{
+    fork_calls = 0;
+    TEST_ASSERT_EQUAL_INT(0, get_available_templates(ENV_PROVIDER));
+    TEST_ASSERT_EQUAL_INT_MESSAGE(
+        0, fork_calls, "a provider call must not fork() (bug-1969)");
+}
+
+/* bug-1969: the script gets the provider environment (PROVIDER_NAME,
+ * PRO_CONF_DIR, PRO_DATA_DIR), while the scaler's own environment is left
+ * alone -- it is passed to the child, not setenv() in a threaded process. */
+static void test_provider_call_passes_environment(void)
+{
+    char key[256];
+
+    TEST_ASSERT_EQUAL_INT(0, get_available_templates(ENV_PROVIDER));
+    my_snprintf(key, sizeof(key), "%s#%s:%s:%s", ENV_PROVIDER, ENV_PROVIDER,
+                "/etc/xce-env-conf", "/tmp");
+    TEST_ASSERT_NOT_NULL_MESSAGE(template_get(&g_scaler_ctx.templates, key),
+                                 "script did not see the provider environment");
+    TEST_ASSERT_NULL(getenv("PROVIDER_NAME"));
+    TEST_ASSERT_NULL(getenv("PRO_CONF_DIR"));
+}
+
 void test_plug_mgr(void)
 {
     global_init();
@@ -166,6 +215,8 @@ void test_plug_mgr(void)
     RUN_TEST(test_get_return_request);
     RUN_TEST(test_trigger_release);
     RUN_TEST(test_get_release_request_status);
+    RUN_TEST(test_provider_call_does_not_fork);
+    RUN_TEST(test_provider_call_passes_environment);
 
     global_cleanup();
     return;
diff --git a/src/autoscaler/scaler/testsuite/test_scale_misc.c b/src/autoscaler/scaler/testsuite/test_scale_misc.c
index 52791b75..114aacd7 100644
--- a/src/autoscaler/scaler/testsuite/test_scale_misc.c
+++ b/src/autoscaler/scaler/testsuite/test_scale_misc.c
@@ -6,7 +6,7 @@
 
 #include "scaler/types.h"
 #include "scaler_misc.h"
-#include "unity.h"
+#include <unity/unity.h>
 
 #ifndef TEST_ASSERT_NOT_EQUAL_STRING
 #define TEST_ASSERT_NOT_EQUAL_STRING(a, b)                                     \