From 8409cc2b633715b4237121271f92bb67eca36572 Mon Sep 17 00:00:00 2001 From: Viktor Szakats Date: Tue, 25 Feb 2025 18:13:38 +0100 Subject: [PATCH] runtests: drop logic calling the `handle` tool (Windows) In the cases observed throughout the last year, `handle64` run once per test run, but with no action (match or task kill). It did not help with flakiness and seems redundant. runtests launched it (if present) in Cygwin/MSYS jobs too, where it probably shouldn't have, because we have seen no flakiness there. In CI the tool was present and launched in MSYS2 jobs, but not in Cygwin. After this patch the "clearlocks" warning remain in the log. They are consistently appearing once in every MSVC CI log, early in the tests: ``` test 3207 SKIPPED: curl lacks OpenSSL support [...START-OF-TESTS...] test 0003...[HTTP POST with auth and contents but with content-length set to 0] --pd---e--- OK (3 out of 1596, remaining: 17:50, took 1.423s, duration: 00:02) test 0007...[HTTP with cookie parser and header recording] --pd--oe--- OK (7 out of 1596, remaining: 07:51, took 1.485s, duration: 00:02) test 0006...[HTTP with simple cookie send] --pd---e--- OK (6 out of 1596, remaining: 09:11, took 1.488s, duration: 00:02) test 0005...[HTTP over proxy] --pd---e--- OK (5 out of 1596, remaining: 11:03, took 1.491s, duration: 00:02) CUSTOMBUILD : error : 169: cleardir(log/8/lock) failed [D:\a\curl\curl\bld\tests\test-ci.vcxproj] test 0001...[HTTP GET] --pd---e--- OK (1 out of 1596, remaining: 55:34, took 1.466s, duration: 00:02) test 0004...[Replaced internal and added custom HTTP headers] ``` Ref: https://github.com/curl/curl/actions/runs/13546192228/job/37858323380?pr=16484#step:14:167 Ref: e53523fef07894991c69d907a7c7794c7ada4ff4 #14859 Ref: 311c31ec8e721b11ba77adf7a3865cf0cd30aa42 #6179 Follow-up to 3a8920e5edaead8304a818594f54485a5564f976 #16600 Closes #16484 --- appveyor.sh | 2 +- tests/CMakeLists.txt | 2 +- tests/Makefile.am | 2 +- tests/runner.pm | 25 +--------------------- tests/runtests.md | 5 ----- tests/runtests.pl | 37 +++++---------------------------- tests/servers.pm | 49 -------------------------------------------- 7 files changed, 9 insertions(+), 113 deletions(-) diff --git a/appveyor.sh b/appveyor.sh index efe870a7fc..b79838367c 100644 --- a/appveyor.sh +++ b/appveyor.sh @@ -158,7 +158,7 @@ if [ "${TFLAGS}" != 'skipall' ] && \ time cmake --build _bld --config "${PRJ_CFG}" --target test-ci else ( - TFLAGS="-a -p !flaky -r -rm ${TFLAGS}" + TFLAGS="-a -p !flaky -r ${TFLAGS}" cd _bld/tests time ./runtests.pl ) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index e3de29d6ac..b03fd53216 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -104,7 +104,7 @@ curl_add_runtests(test-am "-a -am") curl_add_runtests(test-full "-a -p -r") # ~flaky means that it ignores results of tests using the flaky keyword curl_add_runtests(test-nonflaky "-a -p ~flaky ~timing-dependent") -curl_add_runtests(test-ci "-a -p ~flaky ~timing-dependent -r -rm -j20") +curl_add_runtests(test-ci "-a -p ~flaky ~timing-dependent -r -j20") curl_add_runtests(test-torture "-a -t -j20") curl_add_runtests(test-event "-a -e") diff --git a/tests/Makefile.am b/tests/Makefile.am index eec55ab909..05012cb441 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -141,7 +141,7 @@ TEST_E = -a -e TEST_NF = -a -p ~flaky ~timing-dependent # special CI target derived from nonflaky with CI-specific flags -TEST_CI = $(TEST_NF) -r -rm -j20 +TEST_CI = $(TEST_NF) -r -j20 PYTEST = pytest endif diff --git a/tests/runner.pm b/tests/runner.pm index 7f379f42a1..0c48f8ced6 100644 --- a/tests/runner.pm +++ b/tests/runner.pm @@ -47,7 +47,6 @@ BEGIN { readtestkeywords restore_test_env runner_init - runnerac_clearlocks runnerac_shutdown runnerac_stopservers runnerac_test_preprocess @@ -88,7 +87,6 @@ use processhelp qw( ); use servers qw( checkcmd - clearlocks initserverconfig serverfortest stopserver @@ -1272,12 +1270,6 @@ sub runner_test_run { return (0, clearlogs(), \%testtimings, $cmdres, $CURLOUT, $tool, $usedvalgrind); } -# Async call runner_clearlocks -# Called by controller -sub runnerac_clearlocks { - return controlleripccall(\&runner_clearlocks, @_); -} - # Async call runner_shutdown # This call does NOT generate an IPC response and must be the last IPC call # received. @@ -1471,10 +1463,7 @@ sub ipcrecv { # print "ipcrecv $funcname\n"; # Synchronously call the desired function my @res; - if($funcname eq "runner_clearlocks") { - @res = runner_clearlocks(@$argsarrayref); - } - elsif($funcname eq "runner_shutdown") { + if($funcname eq "runner_shutdown") { runner_shutdown(@$argsarrayref); # Special case: no response will be forthcoming return 1; @@ -1508,18 +1497,6 @@ sub ipcrecv { return 0; } -################################################################### -# Kill the server processes that still have lock files in a directory -sub runner_clearlocks { - my ($lockdir)=@_; - if(clearlogs()) { - logmsg "Warning: log messages were lost\n"; - } - clearlocks($lockdir); - return clearlogs(); -} - - ################################################################### # Kill all server processes sub runner_stopservers { diff --git a/tests/runtests.md b/tests/runtests.md index 2c875440d7..60b1c29381 100644 --- a/tests/runtests.md +++ b/tests/runtests.md @@ -219,11 +219,6 @@ Display run time statistics. (Requires the `Perl Time::HiRes` module) Display full run time statistics. (Requires the `Perl Time::HiRes` module) -## `-rm` - -Force removal of files by killing locking processes. (Windows only, requires -the **Sysinternals** `handle[64].exe` to be on PATH) - ## `--repeat=[num]` This repeats the given set of test numbers this many times. If no test numbers diff --git a/tests/runtests.pl b/tests/runtests.pl index f360920b02..81baf2cbe0 100755 --- a/tests/runtests.pl +++ b/tests/runtests.pl @@ -161,7 +161,6 @@ my $globalabort; # flag signalling program abort # values for $singletest_state use constant { ST_INIT => 0, - ST_CLEARLOCKS => 1, ST_INITED => 2, ST_PREPROCESS => 3, ST_RUN => 4, @@ -180,7 +179,6 @@ my %runnersrunning; # tests currently running by runner ID my $short; my $no_debuginfod; my $keepoutfiles; # keep stdout and stderr files after tests -my $clearlocks; # force removal of files by killing locking processes my $postmortem; # display detailed info about failed tests my $run_disabled; # run the specific tests even if listed in DISABLED my $scrambleorder; @@ -1866,33 +1864,13 @@ sub singletest { if($singletest_state{$runnerid} == ST_INIT) { my $logdir = getrunnerlogdir($runnerid); # first, remove all lingering log & lock files - if((!cleardir($logdir) || !cleardir("$logdir/$LOCKDIR")) - && $clearlocks) { - # On Windows, lock files can't be deleted when the process still - # has them open, so kill those processes first - if(runnerac_clearlocks($runnerid, "$logdir/$LOCKDIR")) { - logmsg "ERROR: runner $runnerid seems to have died\n"; - $singletest_state{$runnerid} = ST_INIT; - return (-1, 0); - } - $singletest_state{$runnerid} = ST_CLEARLOCKS; - } else { - $singletest_state{$runnerid} = ST_INITED; - # Recursively call the state machine again because there is no - # event expected that would otherwise trigger a new call. - return singletest(@_); + if(!cleardir($logdir)) { + logmsg "Warning: $runnerid: cleardir($logdir) failed\n"; } - - } elsif($singletest_state{$runnerid} == ST_CLEARLOCKS) { - my ($rid, $logs) = runnerar($runnerid); - if(!$rid) { - logmsg "ERROR: runner $runnerid seems to have died\n"; - $singletest_state{$runnerid} = ST_INIT; - return (-1, 0); + if(!cleardir("$logdir/$LOCKDIR")) { + logmsg "Warning: $runnerid: cleardir($logdir/$LOCKDIR) failed\n"; } - logmsg $logs; - my $logdir = getrunnerlogdir($runnerid); - cleardir($logdir); + $singletest_state{$runnerid} = ST_INITED; # Recursively call the state machine again because there is no # event expected that would otherwise trigger a new call. @@ -2447,10 +2425,6 @@ while(@ARGV) { $fullstats=1; } } - elsif($ARGV[0] eq "-rm") { - # force removal of files by killing locking processes - $clearlocks=1; - } elsif($ARGV[0] eq "-u") { # error instead of warning on server unexpectedly alive $err_unexpected=1; @@ -2483,7 +2457,6 @@ Usage: runtests.pl [options] [test selection(s)] -R scrambled order (uses the random seed, see --seed) -r run time statistics -rf full run time statistics - -rm force removal of files by killing locking processes (Windows only) --repeat=[num] run the given tests this many times -s short output --seed=[num] set the random seed to a fixed number diff --git a/tests/servers.pm b/tests/servers.pm index 7bc9a1273c..7af1383715 100644 --- a/tests/servers.pm +++ b/tests/servers.pm @@ -54,7 +54,6 @@ BEGIN { # functions qw( checkcmd - clearlocks serverfortest stopserver stopservers @@ -262,54 +261,6 @@ sub init_serverpidfile_hash { } -####################################################################### -# Kill the processes that still have lock files in a directory -# -sub clearlocks { - my $dir = $_[0]; - my $done = 0; - - if(os_is_win()) { - $dir = sys_native_abs_path($dir); - # Must use backslashes for handle64 to find a match - if ($^O eq 'MSWin32') { - $dir =~ s/\//\\/g; - } - else { - $dir =~ s/\//\\\\/g; - } - my $handle = "handle"; - if($ENV{"PROCESSOR_ARCHITECTURE"} =~ /64$/) { - $handle = "handle64"; - } - if(checkcmd($handle)) { - # https://learn.microsoft.com/sysinternals/downloads/handle#usage - my $cmd = "$handle $dir -accepteula -nobanner"; - logmsg "clearlocks: Executing query: '$cmd'\n"; - my @handles = `$cmd`; - for my $tryhandle (@handles) { - # Skip the "No matching handles found." warning when returned - if($tryhandle =~ /^(\S+)\s+pid:\s+(\d+)\s+type:\s+(\w+)\s+([0-9A-F]+):\s+(.+)\r\r/) { - logmsg "clearlocks: Found $3 lock of '$5' ($4) by $1 ($2)\n"; - # Ignore stunnel since we cannot do anything about its locks - if("$3" eq "File" && "$1" ne "tstunnel.exe") { - logmsg "clearlocks: Killing IMAGENAME eq $1 and PID eq $2\n"; - # https://ss64.com/nt/taskkill.html - my $cmd = "taskkill.exe -f -t -fi \"IMAGENAME eq $1\" -fi \"PID eq $2\" >nul 2>&1"; - logmsg "clearlocks: Executing kill: '$cmd'\n"; - system($cmd); - $done = 1; - } - } - } - } - else { - logmsg "Warning: 'handle' tool not found.\n"; - } - } - return $done; -} - ####################################################################### # Check if a given child process has just died. Reaps it if so. # -- 2.47.2