From 3dcfdaf531f2ffbebf8c2a555c894212746be99c Mon Sep 17 00:00:00 2001 From: Kushal Pisavadia Date: Thu, 23 Oct 2025 11:53:25 +0100 Subject: [PATCH 1/2] TaskQueueTest: Fix race condition in TaskSignalHandling test Replace `sleep` with `condition_variable` synchronization to ensure the child PID is captured before attempting to kill it. This eliminates timing dependencies. --- unittests/Basic/TaskQueueTest.cpp | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/unittests/Basic/TaskQueueTest.cpp b/unittests/Basic/TaskQueueTest.cpp index 460db418551a4..85085d3f496a7 100644 --- a/unittests/Basic/TaskQueueTest.cpp +++ b/unittests/Basic/TaskQueueTest.cpp @@ -18,6 +18,7 @@ #include #include +#include #include #include @@ -93,9 +94,15 @@ TEST(TaskQueueTest, TaskSignalHandling) { bool TaskSignalled = false; int ReceivedSignal = 0; ProcessId ChildPid = 0; + std::mutex PidMutex; + std::condition_variable PidCv; auto TaskBegan = [&](ProcessId Pid, void *Context) { - ChildPid = Pid; + { + std::lock_guard lock(PidMutex); + ChildPid = Pid; + } + PidCv.notify_one(); }; auto TaskSignalledCallback = [&](ProcessId Pid, llvm::StringRef ErrorMsg, @@ -117,7 +124,11 @@ TEST(TaskQueueTest, TaskSignalHandling) { TQ.execute(TaskBegan, nullptr, TaskSignalledCallback); }); - std::this_thread::sleep_for(std::chrono::milliseconds(100)); + // Wait for the task to actually start and get its PID + { + std::unique_lock lock(PidMutex); + PidCv.wait_for(lock, std::chrono::seconds(5), [&] { return ChildPid > 0; }); + } if (ChildPid > 0) { EXPECT_EQ(0, kill(ChildPid, SIGTERM)) << "Should kill the specific child process we spawned"; From 8f14263fd55b4da31d817e1cd24ddc1aa8008853 Mon Sep 17 00:00:00 2001 From: Kushal Pisavadia Date: Thu, 23 Oct 2025 12:40:57 +0100 Subject: [PATCH 2/2] TaskQueue: Add destructor to close FDs, preventing resource leak The `Task` class was missing a destructor to close pipe file descriptors when destroyed. This caused file descriptor exhaustion after ~2,187 test runs, making tests fail with POLLNVAL errors when the system ran out of resources. This was found with: ``` ./unittests/Basic/SwiftBasicTests --gtest_filter="TaskQueueTest.*" --gtest_repeat=1000 ``` --- lib/Basic/Unix/TaskQueue.inc | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/lib/Basic/Unix/TaskQueue.inc b/lib/Basic/Unix/TaskQueue.inc index 3aaf9ffe93d29..bfd46a68797d6 100644 --- a/lib/Basic/Unix/TaskQueue.inc +++ b/lib/Basic/Unix/TaskQueue.inc @@ -114,6 +114,18 @@ public: "Env must either be empty or null-terminated!"); } + ~Task() { + // Ensure pipes are closed when the task is destroyed + if (Pipe >= 0) { + close(Pipe); + Pipe = -1; + } + if (ErrorPipe >= 0) { + close(ErrorPipe); + ErrorPipe = -1; + } + } + const char *getExecPath() const { return ExecPath; } ArrayRef getArgs() const { return Args; } StringRef getOutput() const { return Output; }