close stream fd

Signed-off-by: dentiny <[email protected]>

src/ray/util/stream_redirection_utils.cc

@@ -42,7 +42,8 @@ absl::flat_hash_map<int, RedirectionFileHandle> redirection_file_handles;
 // ONCE** at program termination.
 std::once_flag stream_exit_once_flag;
 void SyncOnStreamRedirection() {
-  for (auto &[_, handle] : redirection_file_handles) {
+  for (auto &[stream_fd, handle] : redirection_file_handles) {
+    RAY_CHECK_OK(Close(stream_fd));
     handle.Close();
   }
 }

src/ray/util/tests/BUILD

@@ -257,6 +257,26 @@ ray_cc_test(
     ],
 )
 
+ray_cc_test(
+    name = "stream_redirection_exit_test",
+    srcs = ["stream_redirection_exit_test.cc"],
+    deps = [
+        "//src/ray/common/test:testing",
+        "//src/ray/util",
+        "//src/ray/util:stream_redirection_utils",
+        "@com_google_googletest//:gtest_main",
+    ],
+    size = "small",
+    tags = [
+        "team:core",
+        # TSAN fails to understand synchroization logic, from the stacktrace, it shows we flush
+        # ostream concurrently at pipe dumper thread and main thread, which we have ordered
+        # properly. Disable the complete test suite here since it always contains exactly one test
+        # case.
+        "no_tsan",
+    ],
+)
+
 ray_cc_test(
     name = "cmd_line_utils_test",
     srcs = ["cmd_line_utils_test.cc"],

src/ray/util/tests/stream_redirection_exit_test.cc

@@ -0,0 +1,58 @@
+// Copyright 2025 The Ray Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//  http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// This test case only checks whether stream redirection process could exit normally.
+
+#include <gtest/gtest.h>
+
+#include <chrono>
+#include <iostream>
+#include <thread>
+
+#include "ray/common/test/testing.h"
+#include "ray/util/filesystem.h"
+#include "ray/util/stream_redirection_utils.h"
+#include "ray/util/util.h"
+
+namespace ray {
+
+namespace {
+constexpr std::string_view kLogLine1 = "hello\n";
+constexpr std::string_view kLogLine2 = "world";
+}  // namespace
+
+TEST(LoggingUtilTest, RedirectStderr) {
+  const std::string test_file_path = absl::StrFormat("%s.err", GenerateUUIDV4());
+
+  // Works via `dup`, so have to execute before we redirect via `dup2` and close stderr.
+  testing::internal::CaptureStderr();
+
+  // Redirect stderr for testing, so we could have stdout for debugging.
+  StreamRedirectionOption opts;
+  opts.file_path = test_file_path;
+  opts.tee_to_stderr = true;
+  RedirectStderr(opts);
+
+  std::cerr << kLogLine1 << std::flush;
+  std::cerr << kLogLine2 << std::flush;
+
+  // TODO(hjiang): Current implementation is flaky intrinsically, sleep for a while to
+  // make sure pipe content has been read over to spdlog.
+  std::this_thread::sleep_for(std::chrono::seconds(2));
+  FlushOnRedirectedStderr();
+
+  // Make sure flush hook works fine and process terminates with no problem.
+}
+
+}  // namespace ray