Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merge-on-Red] - Implement Test Process Watcher #78742

Merged
merged 35 commits into from
Mar 10, 2023
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
be9867e
Started the draft for the watch tower.
ivdiazsa Nov 10, 2022
e79ae66
Parsing command-line arguments done.
ivdiazsa Nov 11, 2022
b17eef2
Removed auto-generated file.
ivdiazsa Nov 11, 2022
d40ad9e
Added an example I made on how to run processes from C/C++ :)
ivdiazsa Nov 12, 2022
6cc9e9e
Perfect arg parsing done!
ivdiazsa Nov 16, 2022
fcd9f1a
Seems to be working?
ivdiazsa Nov 16, 2022
1c7f46c
Watchdog is functional now!
ivdiazsa Nov 17, 2022
ec90d1e
Fixed a nuisance with the Windows compiler messing with Linux code, b…
ivdiazsa Nov 22, 2022
b157904
Got the Windows draft working, at long last.
ivdiazsa Nov 23, 2022
36a75f0
Added initial linking of the watcher to the repo's build scripts. Not…
ivdiazsa Dec 6, 2022
c4f2009
Moved the watcher to the CoreCLR project. Not yet functional though...
ivdiazsa Dec 6, 2022
e8b13a5
Got it to build on Windows!
ivdiazsa Dec 7, 2022
b476ad2
Builds and Works on Linux!
ivdiazsa Dec 7, 2022
f74fb2e
Fixed a slowdown with the watcher.
ivdiazsa Dec 8, 2022
958dc29
Forgot to not leak memory :)
ivdiazsa Dec 8, 2022
67e25fe
This is C++, not C.
ivdiazsa Dec 8, 2022
7ef09b2
Bash scripts now call the watcher, but watcher still crashes when bui…
ivdiazsa Dec 14, 2022
226ea36
Renamed variable.
ivdiazsa Dec 14, 2022
5fbd787
FIXED THE LINUX PART!
ivdiazsa Dec 14, 2022
0ff6b8d
Started implementing the constant flow of logs. Had to save my progress.
ivdiazsa Dec 16, 2022
8f21b9c
TEMP LOG BUILDS!
ivdiazsa Dec 17, 2022
c7fc541
TEMP LOGS WORK!
ivdiazsa Dec 17, 2022
4f658e2
Updated with main.
ivdiazsa Feb 15, 2023
a88e48c
Test Watcher seems to be working on Linux and MacOS.
ivdiazsa Feb 22, 2023
8fceb44
Updated the Helix Commands with the watcher now included, and removed…
ivdiazsa Feb 22, 2023
064b9b0
Fixed build issue in the watcher's Windows version.
ivdiazsa Feb 22, 2023
2cb9270
Apparently works on Windows now too?
ivdiazsa Feb 23, 2023
450eba9
Fixed build issue with Windows x86.
ivdiazsa Feb 23, 2023
eba2bcd
Fixed issue where we were not building the watcher in certain subsets…
ivdiazsa Feb 24, 2023
d68d16c
Fixed yet another Windows issue.
ivdiazsa Feb 27, 2023
998965c
Used ExeSuffix instead of recomputing it and excluded Browser-Wasm
ivdiazsa Feb 27, 2023
7cb4e89
Changed the Helix Correlation Payloads to bundle the whole watchdog d…
ivdiazsa Mar 1, 2023
2ba916a
Changed the scripts to use the test watcher in the Core_Root, instead…
ivdiazsa Mar 3, 2023
a712352
Fixed undeclared variable in Windows test scripts.
ivdiazsa Mar 3, 2023
0cc8875
Addressed PR feedback comments.
ivdiazsa Mar 10, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/coreclr/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,11 @@ else()
endif()
endif()

#----------------------------------------------------
# Build the test watchdog alongside the CLR
#----------------------------------------------------
add_subdirectory("${CLR_SRC_NATIVE_DIR}/watchdog" test-watchdog)

# Add this subdir. We install the headers for the jit.
add_subdirectory(pal/prebuilt/inc)

Expand Down Expand Up @@ -264,3 +269,4 @@ endif(CLR_CMAKE_HOST_WIN32)
if(CLR_CROSS_COMPONENTS_BUILD)
include(crosscomponents.cmake)
endif(CLR_CROSS_COMPONENTS_BUILD)

4 changes: 4 additions & 0 deletions src/native/watchdog/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
add_executable_clr(watchdog ${CMAKE_CURRENT_LIST_DIR}/watchdog.cpp)
install_clr(TARGETS watchdog DESTINATIONS . COMPONENT hosts)
install_clr(TARGETS watchdog DESTINATIONS . COMPONENT nativeaot)

139 changes: 139 additions & 0 deletions src/native/watchdog/watchdog.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#include <cstdio>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Add license header

#include <cstdlib>
#include <errno.h>
#include <signal.h>

#ifdef TARGET_WINDOWS

#include <windows.h>
#include <string>

#else // !TARGET_WINDOWS

#include <chrono>
#include <sys/wait.h>
#include <thread>
#include <unistd.h>
#include <vector>

#endif // TARGET_WINDOWS

int run_timed_process(const long, const int, const char *[]);

#ifdef TARGET_X86
int __cdecl main(const int argc, const char *argv[])
#else
int main(const int argc, const char *argv[])
#endif
{
if (argc < 3)
{
printf("There are missing arguments. Got %d instead of 3+ :(\n", argc);
return EXIT_FAILURE;
}

const long timeout_sec = strtol(argv[1], nullptr, 10);
int exit_code = run_timed_process(timeout_sec * 1000L, argc-2, &argv[2]);

printf("App Exit Code: %d\n", exit_code);
return exit_code;
}

int run_timed_process(const long timeout_ms, const int proc_argc, const char *proc_argv[])
{
#ifdef TARGET_WINDOWS
std::string cmdline(proc_argv[0]);

for (int i = 1; i < proc_argc; i++)
{
cmdline.append(" ");
cmdline.append(proc_argv[i]);
}

STARTUPINFOA startup_info;
PROCESS_INFORMATION proc_info;
unsigned long exit_code;

ZeroMemory(&startup_info, sizeof(startup_info));
startup_info.cb = sizeof(startup_info);
ZeroMemory(&proc_info, sizeof(proc_info));

if (!CreateProcessA(NULL, &cmdline[0], NULL, NULL, FALSE, 0, NULL, NULL,
&startup_info, &proc_info))
{
int error_code = GetLastError();
printf("Process creation failed... Code %d.\n", error_code);
return error_code;
}

WaitForSingleObject(proc_info.hProcess, timeout_ms);
GetExitCodeProcess(proc_info.hProcess, &exit_code);

CloseHandle(proc_info.hProcess);
CloseHandle(proc_info.hThread);
return exit_code;

#else // !TARGET_WINDOWS

// TODO: Describe what the 'ms_factor' is, and why it's being used here.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Address this TODO?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops forgot that. Thanks for the catch.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually removed it altogether. I needed it because originally, I was dealing with some C stuff that combined microseconds and milliseconds. After switching to C++'s std::chrono::milliseconds, testing with and without it yielded virtually the same wait time, so it ended up being redundant.

const int ms_factor = 40;
const int check_interval = 1000 / ms_factor;

int check_count = 0;
std::vector<const char*> args;

pid_t child_pid;
int child_status;
int wait_code;

for (int i = 0; i < proc_argc; i++)
{
args.push_back(proc_argv[i]);
}
args.push_back(NULL);

child_pid = fork();

if (child_pid < 0)
{
// Fork failed. No memory remaining available :(
printf("Fork failed... Returning ENOMEM.\n");
return ENOMEM;
}
else if (child_pid == 0)
{
// Instructions for child process!
execv(args[0], const_cast<char* const*>(args.data()));
}
else
{
do
{
// Instructions for the parent process!
wait_code = waitpid(child_pid, &child_status, WNOHANG);

if (wait_code == -1)
return EINVAL;

std::this_thread::sleep_for(std::chrono::milliseconds(check_interval));

if (wait_code)
{
if (WIFEXITED(child_status))
return WEXITSTATUS(child_status);
}
check_count += ms_factor;

} while (check_count < ((timeout_ms / check_interval) * ms_factor));
}

printf("Child process took too long. Timed out... Exiting...\n");
kill(child_pid, SIGKILL);

#endif // TARGET_WINDOWS
return ETIMEDOUT;
}

32 changes: 27 additions & 5 deletions src/tests/Common/CLRTest.Execute.Bash.targets
Original file line number Diff line number Diff line change
Expand Up @@ -180,14 +180,27 @@ fi
<Description>Set CORE_ROOT to the specified value before running the test.</Description>
</BashCLRTestExecutionScriptArgument>

<BashCLRTestExecutionScriptArgument Include="watcher">
<HasParam>true</HasParam>
<ParamText>=*</ParamText> <!-- Bash specific -->
<ParamName>watcherFullPath</ParamName>
<Command><![CDATA[ export _WatcherFullPath="${i#*=}"
if [ ! -f "$_WatcherFullPath" ]
then
echo "The Watcher FullPath %5C%22${_WatcherFullPath}%5C%22 does not exist"
usage
fi]]></Command>
<Description>Path to the test watcher that will look over the test.</Description>
</BashCLRTestExecutionScriptArgument>

<BashCLRTestExecutionScriptArgument Include="e;env">
<HasParam>true</HasParam>
<ParamText>=*</ParamText> <!-- Bash specific -->
<ParamName>dotenvFullPath</ParamName>
<Command><![CDATA[ export __DotEnv="${i#*=}"
if [ ! -f "$__DotEnv" ]
then
echo "The Debugger FullPath %5C%22${__DotEnv}%5C%22 does not exist"
echo "The dotenv file FullPath %5C%22${__DotEnv}%5C%22 does not exist"
usage
fi
export __DotEnvArg=-e ${__DotEnv}]]></Command>
Expand Down Expand Up @@ -250,10 +263,11 @@ then
exit 1
fi

# Copy CORECLR native binaries to $LinkBin,
# Copy CORECLR native binaries and the test watcher (if provided) to $LinkBin,
# so that we can run the test based on that directory
cp $CORE_ROOT/*.so $LinkBin/
cp $CORE_ROOT/corerun $LinkBin/
cp $_WatcherFullPath $LinkBin/

# Copy some files that may be arguments
for f in *.txt;
Expand All @@ -263,6 +277,7 @@ then

ExePath=$LinkBin/$(InputAssemblyName)
export CORE_ROOT=$PWD/$LinkBin
export _WatcherFullPath=$PWD/$LinkBin/watchdog
fi
]]>
</BashLinkerTestLaunchCmds>
Expand All @@ -283,6 +298,7 @@ fi
</PropertyGroup>
<PropertyGroup>
<CLRTestRunFile Condition="'$(CLRTestIsHosted)'=='true'">"$CORE_ROOT/corerun" $(CoreRunArgs) ${__DotEnvArg}</CLRTestRunFile>
<WatcherRunFile>"$_WatcherFullPath" 300</WatcherRunFile>

<!-- Note that this overwrites CLRTestBashPreCommands rather than adding to it. -->
<CLRTestBashPreCommands Condition="'$(CLRTestKind)' == 'BuildAndRun' and '$(TargetArchitecture)' == 'wasm'"><![CDATA[
Expand Down Expand Up @@ -318,8 +334,11 @@ fi
if [ ! -z "$CLRCustomTestLauncher" ];
then
LAUNCHER="$CLRCustomTestLauncher $PWD/"
else
elif [ ! -z "$_DebuggerFullPath" ]
then
LAUNCHER="$_DebuggerFullPath $_DebuggerArgsSeparator $(CLRTestRunFile)"
else
LAUNCHER="$(WatcherRunFile) $(CLRTestRunFile)"
fi

$(BashIlrtTestLaunchCmds)
Expand All @@ -346,8 +365,11 @@ $(BashLinkerTestLaunchCmds)
if [ ! -z "$CLRCustomTestLauncher" ];
then
LAUNCHER="$CLRCustomTestLauncher $PWD/"
elif [ ! -z "$_DebuggerFullPath" ]
then
LAUNCHER="$_DebuggerFullPath $_DebuggerArgsSeparator $(CLRTestRunFile)"
else
LAUNCHER="$_DebuggerFullPath $(CLRTestRunFile)"
LAUNCHER="$(WatcherRunFile) $(CLRTestRunFile)"
fi

$(BashIlrtTestLaunchCmds)
Expand Down Expand Up @@ -484,7 +506,7 @@ usage()
for i in "$@"
do
case $i in
-?|-h|--help)
-?|-h|--help|/?|/h|/help)
usage
%3B%3B
@(BashCLRTestExecutionScriptArgument -> ' -%(Identity)%(ParamText)|/%(Identity)%(ParamText))
Expand Down
26 changes: 23 additions & 3 deletions src/tests/Common/CLRTest.Execute.Batch.targets
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,20 @@ Exit /b 0
]]></Command>
<Description>Set CORE_ROOT to the specified value before running the test.</Description>
</BatchCLRTestExecutionScriptArgument>

<BatchCLRTestExecutionScriptArgument Include="watcher">
<HasParam>true</HasParam>
<ParamName>watcherFullPath</ParamName>
<Command><![CDATA[
IF EXIST "%2" (
set _WatcherFullPath=%2
) ELSE (
ECHO The Watcher FullPath "%2" does not exist
GOTO :USAGE
)
]]></Command>
<Description>Path to the test watcher that will look over the test.</Description>
</BatchCLRTestExecutionScriptArgument>
</ItemGroup>

<PropertyGroup>
Expand Down Expand Up @@ -260,17 +274,19 @@ IF defined DoLink (
Exit /b 1
)

REM Copy CORECLR native binaries to %LinkBin%, so that we can run the test based on that directory
REM Copy CORECLR native binaries and the test watcher (if provided) to %LinkBin%, so that we can run the test based on that directory
copy %CORE_ROOT%\clrjit.dll %LinkBin% > nul 2> nul
copy %CORE_ROOT%\coreclr.dll %LinkBin% > nul 2> nul
copy %CORE_ROOT%\mscorrc.dll %LinkBin% > nul 2> nul
copy %CORE_ROOT%\CoreRun.exe %LinkBin% > nul 2> nul
copy %_WatcherFullPath% %LinkBin% > nul 2> nul

REM Copy some files that may be arguments
copy *.txt %LinkBin% > nul 2> nul

set ExePath=%LinkBin%\$(InputAssemblyName)
set CORE_ROOT=%scriptPath%LinkBin%
set CORE_ROOT=%scriptPath%\%LinkBin%
set _WatcherFullPath=%scriptPath%\%LinkBin%\watchdog.exe
)
]]>
</BatchLinkerTestLaunchCmds>
Expand All @@ -289,6 +305,8 @@ if defined DoLink (
</PropertyGroup>
<PropertyGroup>
<CLRTestRunFile Condition="'$(CLRTestIsHosted)'=='true'">"%CORE_ROOT%\corerun.exe" $(CoreRunArgs) %__DotEnvArg%</CLRTestRunFile>
<WatcherRunFile>"%_WatcherFullPath%" 300</WatcherRunFile>

<BatchCopyCoreShimLocalCmds Condition="'$(CLRTestScriptLocalCoreShim)' == 'true'"><![CDATA[
REM Local CoreShim requested - see MSBuild property 'CLRTestScriptLocalCoreShim'
ECHO Copying '%CORE_ROOT%\CoreShim.dll'...
Expand All @@ -301,8 +319,10 @@ $(BatchCopyCoreShimLocalCmds)

IF NOT "%CLRCustomTestLauncher%"=="" (
set LAUNCHER=call %CLRCustomTestLauncher% %scriptPath%
) ELSE (
) ELSE IF NOT "%_DebuggerFullPath%"=="" (
set LAUNCHER=%_DebuggerFullPath% $(CLRTestRunFile)
) ELSE (
set LAUNCHER=$(WatcherRunFile) $(CLRTestRunFile)
)

$(BatchIlrtTestLaunchCmds)
Expand Down
16 changes: 14 additions & 2 deletions src/tests/Common/helixpublishwitharcade.proj
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,9 @@

<!-- On Windows, you need to "call" a script or else the cmd.exe instance it is running in will exit, and the HelixPostCommands will not execute. -->
<_MergedWrapperRunScriptPrefix Condition="'$(TestWrapperTargetsWindows)' == 'true'">call </_MergedWrapperRunScriptPrefix>

<_MergedWrapperRunScriptWatcherArgs Condition="'$(TestWrapperTargetsWindows)' != 'true'">-watcher=./watchdog</_MergedWrapperRunScriptWatcherArgs>
<_MergedWrapperRunScriptWatcherArgs Condition="'$(TestWrapperTargetsWindows)' == 'true'">/watcher=.\watchdog.exe</_MergedWrapperRunScriptWatcherArgs>
</PropertyGroup>

<ItemGroup Condition="'@(_MergedWrapperMarker)' != ''" >
Expand Down Expand Up @@ -411,8 +414,8 @@
<HelixCommandLines Condition="'$(TestWrapperTargetsWindows)' != 'true'" Include="export TEST_HARNESS_STRIPE_TO_EXECUTE=.0.1" />
<HelixCommandLines Condition="'$(TestWrapperTargetsWindows)' != 'true'" Include="chmod +x $(_MergedWrapperRunScriptRelative)" />
<!-- Force assemblies to lazy-load for LLVM AOT test runs to enable using tests that fail at AOT time (and as a result can't be AOTd) -->
<HelixCommandLines Condition="'$(RuntimeVariant)' == 'llvmfullaot'" Include="$(_MergedWrapperRunScriptPrefix)$(_MergedWrapperRunScriptRelative) --aot-lazy-assembly-load" />
<HelixCommandLines Condition="'$(RuntimeVariant)' != 'llvmfullaot'" Include="$(_MergedWrapperRunScriptPrefix)$(_MergedWrapperRunScriptRelative)" />
<HelixCommandLines Condition="'$(RuntimeVariant)' == 'llvmfullaot'" Include="$(_MergedWrapperRunScriptPrefix)$(_MergedWrapperRunScriptRelative) $(_MergedWrapperRunScriptWatcherArgs) --aot-lazy-assembly-load" />
<HelixCommandLines Condition="'$(RuntimeVariant)' != 'llvmfullaot'" Include="$(_MergedWrapperRunScriptPrefix)$(_MergedWrapperRunScriptRelative) $(_MergedWrapperRunScriptWatcherArgs)" />
<HelixCommandLines Include="$(XUnitLogCheckerCommand)" />
</ItemGroup>

Expand Down Expand Up @@ -718,9 +721,18 @@
<XUnitRunnerDll>$CORE_ROOT/xunit/xunit.console.dll</XUnitRunnerDll>
</PropertyGroup>

<PropertyGroup>
<TestWatcherExe Condition="'$(TestWrapperTargetsWindows)' != 'true'">watchdog</TestWatcherExe>
<TestWatcherExe Condition="'$(TestWrapperTargetsWindows)' == 'true'">watchdog.exe</TestWatcherExe>
jkoritzinsky marked this conversation as resolved.
Show resolved Hide resolved

<TestWatcherPath>$(ArtifactsDir)obj/coreclr/$(OSPlatformConfig)/test-watchdog/$(TestWatcherExe)</TestWatcherPath>
<TestWatcherPath>$([MSBuild]::NormalizePath($(TestWatcherPath)))</TestWatcherPath>
</PropertyGroup>

<ItemGroup Condition=" '$(UsesHelixSdk)' == 'true' ">
<HelixCorrelationPayload Include="$(CoreRootDirectory)" />
<HelixCorrelationPayload Include="$(XUnitLogCheckerDirectory)" />
<HelixCorrelationPayload Include="$(TestWatcherPath)" />

<LegacyPayloads Include="$([System.IO.Directory]::GetDirectories($(LegacyPayloadsRootDirectory)))" Condition="Exists('$(LegacyPayloadsRootDirectory)')" />
<LegacyPayloads Update="@(LegacyPayloads)">
Expand Down