From e4d0169107b6359946997b06bbc2cdd4f119bf16 Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 13 Jun 2024 15:23:48 -0400 Subject: ctest: Fix spurious build failures with CTEST_USE_LAUNCHERS on Windows Remove the stdio handle inheritance suppression originally added by commit f262298bb0 (... do not inherit pipes in child procs for ctest so it can kill them, 2007-09-11, v2.6.0~1136). It's not clear what problem it was trying to solve, was only done in `ctest` and not `cmake`, and since commit 9c3ffe2474 (BUG: fix problem with stdout and stderr not showing up in ms dos shells, 2007-09-25, v2.6.0~1066) has not been done in `ctest` launched under interactive consoles. Furthermore, the code has been spuriously breaking stdio when `ctest` is started with both stdout and stderr connected to the same pipe, such as when `ctest --launch` is used under `ninja`. This is because it used `DuplicateHandle` with `DUPLICATE_CLOSE_SOURCE` on the stdout handle and then the stderr handle. If the handles are the same, then the stderr handle becomes invalid in between these operations, leading to likely-undefined behavior. Since commit 96b3dd329e (cmCTestLaunchReporter: Replace cmsysProcess with cmUVProcessChain, 2023-07-26, v3.28.0-rc1~138^2~2) this became more noticeable because `uv_spawn` performs additional verification on stdio handles. This could be fixed by instead suppressing inheritance via SetHandleInformation(h, HANDLE_FLAG_INHERIT, 0); However, the functionality no longer seems necessary, so remove it. --- Source/cmSystemTools.cxx | 27 --------------------------- Source/cmSystemTools.h | 6 ------ Source/ctest.cxx | 1 - 3 files changed, 34 deletions(-) diff --git a/Source/cmSystemTools.cxx b/Source/cmSystemTools.cxx index 2bdc928..9945305 100644 --- a/Source/cmSystemTools.cxx +++ b/Source/cmSystemTools.cxx @@ -2375,33 +2375,6 @@ void cmSystemTools::EnsureStdPipes() } #endif -void cmSystemTools::DoNotInheritStdPipes() -{ -#ifdef _WIN32 - // Check to see if we are attached to a console - // if so, then do not stop the inherited pipes - // or stdout and stderr will not show up in dos - // shell windows - CONSOLE_SCREEN_BUFFER_INFO hOutInfo; - HANDLE hOut = GetStdHandle(STD_OUTPUT_HANDLE); - if (GetConsoleScreenBufferInfo(hOut, &hOutInfo)) { - return; - } - { - HANDLE out = GetStdHandle(STD_OUTPUT_HANDLE); - DuplicateHandle(GetCurrentProcess(), out, GetCurrentProcess(), &out, 0, - FALSE, DUPLICATE_SAME_ACCESS | DUPLICATE_CLOSE_SOURCE); - SetStdHandle(STD_OUTPUT_HANDLE, out); - } - { - HANDLE out = GetStdHandle(STD_ERROR_HANDLE); - DuplicateHandle(GetCurrentProcess(), out, GetCurrentProcess(), &out, 0, - FALSE, DUPLICATE_SAME_ACCESS | DUPLICATE_CLOSE_SOURCE); - SetStdHandle(STD_ERROR_HANDLE, out); - } -#endif -} - #ifdef _WIN32 # ifndef CRYPT_SILENT # define CRYPT_SILENT 0x40 /* Not defined by VS 6 version of header. */ diff --git a/Source/cmSystemTools.h b/Source/cmSystemTools.h index 9563fd6..0f96500 100644 --- a/Source/cmSystemTools.h +++ b/Source/cmSystemTools.h @@ -494,12 +494,6 @@ public: const std::vector& files, cmTarExtractTimestamps extractTimestamps, bool verbose); - // This should be called first thing in main - // it will keep child processes from inheriting the - // stdin and stdout of this process. This is important - // if you want to be able to kill child processes and - // not get stuck waiting for all the output on the pipes. - static void DoNotInheritStdPipes(); static void EnsureStdPipes(); diff --git a/Source/ctest.cxx b/Source/ctest.cxx index fa38a65..55defe7 100644 --- a/Source/ctest.cxx +++ b/Source/ctest.cxx @@ -173,7 +173,6 @@ int main(int argc, char const* const* argv) argc = encoding_args.argc(); argv = encoding_args.argv(); - cmSystemTools::DoNotInheritStdPipes(); cmSystemTools::InitializeLibUV(); cmSystemTools::FindCMakeResources(argv[0]); -- cgit v0.12