From e70c22b6226badf167463029a2e1c3fe44095a64 Mon Sep 17 00:00:00 2001 From: Stefan Eissing Date: Mon, 9 Sep 2024 13:52:44 +0200 Subject: [PATCH] tests: tweak lock file handling and timers - when server are killed by a test case, do not wait for the server lock file to go away. These tests are mostly about client timeouts and the server will hang until killed. - when killing a server successfully, check for a remaining lock file, log its existence and remove it. - lower the delay timings on SLOWDOWN by half - add SLOWDOWNDATA server command to only slow down the FTP data bytes, not the control ones. - lower some timeout values Closes #14835 --- tests/FILEFORMAT.md | 2 ++ tests/data/test1086 | 8 +++--- tests/data/test1112 | 8 +++--- tests/data/test29 | 3 +++ tests/ftpserver.pl | 16 ++++++++---- tests/pathhelp.pm | 3 ++- tests/processhelp.pm | 2 ++ tests/runner.pm | 18 ++++++++------ tests/server/sws.c | 49 +++++++++++++++++++++++++++++++++++- tests/servers.pm | 59 +++++++++++++++++++++++++++++++------------- 10 files changed, 126 insertions(+), 42 deletions(-) diff --git a/tests/FILEFORMAT.md b/tests/FILEFORMAT.md index a9ae97ff0..cbf2ac339 100644 --- a/tests/FILEFORMAT.md +++ b/tests/FILEFORMAT.md @@ -340,6 +340,8 @@ issue. - `RETRSIZE [size]` - Force RETR response to contain the specified size - `NOSAVE` - Do not actually save what is received - `SLOWDOWN` - Send FTP responses with 0.01 sec delay between each byte +- `SLOWDOWNDATA` - Send FTP responses with 0.01 sec delay between each data + byte - `PASVBADIP` - makes PASV send back an illegal IP in its 227 response - `CAPA [capabilities]` - Enables support for and specifies a list of space separated capabilities to return to the client for the IMAP `CAPABILITY`, diff --git a/tests/data/test1086 b/tests/data/test1086 index 0b4b875d9..ae5e7a093 100644 --- a/tests/data/test1086 +++ b/tests/data/test1086 @@ -6,7 +6,7 @@ EPSV RETR timeout FAILURE -SLOWDOWN +SLOWDOWNDATA flaky timing-dependent @@ -14,10 +14,8 @@ timing-dependent # Server-side -# Overload some standard FTP responses to make them shorter and faster -# to avoid wasting time waiting for the data phase to start -SLOWDOWN +SLOWDOWNDATA REPLY welcome 220 Hey REPLY USER 331 OK REPLY PASS 230 OK @@ -90,7 +88,7 @@ ftp FTP download with strict timeout and slow data transfer -ftp://%HOSTIP:%FTPPORT/%TESTNUMBER -m 7 +ftp://%HOSTIP:%FTPPORT/%TESTNUMBER -m 5 diff --git a/tests/data/test1112 b/tests/data/test1112 index b0a610660..5e03509b0 100644 --- a/tests/data/test1112 +++ b/tests/data/test1112 @@ -7,17 +7,15 @@ EPSV RETR timeout FAILURE -SLOWDOWN +SLOWDOWNDATA timing-dependent # Server-side -# Overload some standard FTP responses to make them shorter and faster -# to avoid wasting time waiting for the data phase to start -SLOWDOWN +SLOWDOWNDATA REPLY welcome 220 Hey REPLY USER 331 OK REPLY PASS 230 OK @@ -93,7 +91,7 @@ ftps FTPS download with strict timeout and slow data transfer --k --ftp-ssl-control ftps://%HOSTIP:%FTPSPORT/%TESTNUMBER -m 16 +-k --ftp-ssl-control ftps://%HOSTIP:%FTPSPORT/%TESTNUMBER -m 5 diff --git a/tests/data/test29 b/tests/data/test29 index 64d5e6c6b..689ac43ee 100644 --- a/tests/data/test29 +++ b/tests/data/test29 @@ -26,6 +26,9 @@ wait 10 http + +http + HTTP with 2 secs timeout diff --git a/tests/ftpserver.pl b/tests/ftpserver.pl index de3436b96..abe04959d 100755 --- a/tests/ftpserver.pl +++ b/tests/ftpserver.pl @@ -488,7 +488,7 @@ sub sendcontrol { for(@a) { sockfilt $_; - portable_sleep(0.01); + portable_sleep($ctrldelay); } } my $log; @@ -525,7 +525,7 @@ sub senddata { # pause between each byte for (split(//,$l)) { sockfiltsecondary $_; - portable_sleep(0.01); + portable_sleep($datadelay); } } } @@ -2808,6 +2808,7 @@ sub nodataconn_str { # On success returns 1, otherwise zero. # sub customize { + my($cmdfile) = @_; $ctrldelay = 0; # default is no throttling of the ctrl stream $datadelay = 0; # default is no throttling of the data stream $retrweirdo = 0; # default is no use of RETRWEIRDO @@ -2867,10 +2868,15 @@ sub customize { logmsg "FTPD: read POSTFETCH header data\n"; $postfetch = $1; } + elsif($_ =~ /SLOWDOWNDATA/) { + $ctrldelay=0; + $datadelay=0.005; + logmsg "FTPD: send response data with 5ms delay per byte\n"; + } elsif($_ =~ /SLOWDOWN/) { - $ctrldelay=1; - $datadelay=1; - logmsg "FTPD: send response with 0.01 sec delay between each byte\n"; + $ctrldelay=0.005; + $datadelay=0.005; + logmsg "FTPD: send response with 5ms delay between each byte\n"; } elsif($_ =~ /RETRWEIRDO/) { logmsg "FTPD: instructed to use RETRWEIRDO\n"; diff --git a/tests/pathhelp.pm b/tests/pathhelp.pm index 31de8ff4e..34961f2c1 100644 --- a/tests/pathhelp.pm +++ b/tests/pathhelp.pm @@ -326,7 +326,8 @@ sub sys_native_abs_path { # prevent transforming and may be not processed. $path =~ s{^([\\/])[\\/]+}{$1}g; - print "Inter result: \"$path\"\n"; + # some debugging? enable on need + # print "Inter result: \"$path\"\n"; # Use 'cygpath', '-m' means Windows path with forward slashes, # '-a' means absolute path chomp($path = `cygpath -m -a '$path'`); diff --git a/tests/processhelp.pm b/tests/processhelp.pm index 48f385040..253fa8b34 100644 --- a/tests/processhelp.pm +++ b/tests/processhelp.pm @@ -327,6 +327,8 @@ sub killpid { } } last if(not scalar(@signalled)); + # give any zombies of us a chance to move on to the afterlife + pidwait(0, &WNOHANG); portable_sleep(0.05); } } diff --git a/tests/runner.pm b/tests/runner.pm index 105009d99..53c98bb7e 100644 --- a/tests/runner.pm +++ b/tests/runner.pm @@ -1066,8 +1066,6 @@ sub singletest_clean { } } - waitlockunlock($serverlogslocktimeout); - # Test harness ssh server does not have this synchronization mechanism, # this implies that some ssh server based tests might need a small delay # once that the client command has run to avoid false test failures. @@ -1085,12 +1083,6 @@ sub singletest_clean { portable_sleep($postcommanddelay) if($postcommanddelay); - # timestamp removal of server logs advisor read lock - $$testtimings{"timesrvrlog"} = Time::HiRes::time(); - - # test definition might instruct to stop some servers - # stop also all servers relative to the given one - my @killtestservers = getpart("client", "killserver"); if(@killtestservers) { foreach my $server (@killtestservers) { @@ -1101,6 +1093,16 @@ sub singletest_clean { } } } + + # wait for any servers left running to release their locks + waitlockunlock($serverlogslocktimeout); + + # timestamp removal of server logs advisor read lock + $$testtimings{"timesrvrlog"} = Time::HiRes::time(); + + # test definition might instruct to stop some servers + # stop also all servers relative to the given one + return 0; } diff --git a/tests/server/sws.c b/tests/server/sws.c index c8e9fde68..f8ec7b1e9 100644 --- a/tests/server/sws.c +++ b/tests/server/sws.c @@ -1319,6 +1319,17 @@ static curl_socket_t connect_to(const char *ipaddr, unsigned short port) } #endif + /* We want to do the connect() in a non-blocking mode, since + * Windows has an internal retry logic that may lead to long + * timeouts if the peer is not listening. */ + if(0 != curlx_nonblock(serverfd, TRUE)) { + error = SOCKERRNO; + logmsg("curlx_nonblock(TRUE) failed with error: (%d) %s", + error, sstrerror(error)); + sclose(serverfd); + return CURL_SOCKET_BAD; + } + switch(socket_domain) { case AF_INET: memset(&serveraddr.sa4, 0, sizeof(serveraddr.sa4)); @@ -1360,15 +1371,51 @@ static curl_socket_t connect_to(const char *ipaddr, unsigned short port) if(rc) { error = SOCKERRNO; + if((error == EINPROGRESS) || (error == EWOULDBLOCK)) { + fd_set output; + struct timeval timeout = {1, 0}; /* 1000 ms */ + + FD_ZERO(&output); + FD_SET(serverfd, &output); + while(1) { + rc = select((int)serverfd + 1, NULL, &output, NULL, &timeout); + if(rc < 0 && SOCKERRNO != EINTR) + goto error; + else if(rc > 0) { + curl_socklen_t errSize = sizeof(error); + if(0 != getsockopt(serverfd, SOL_SOCKET, SO_ERROR, + (void *)&error, &errSize)) + error = SOCKERRNO; + if((0 == error) || (EISCONN == error)) + goto success; + else if((error != EINPROGRESS) && (error != EWOULDBLOCK)) + goto error; + } + else if(!rc) { + logmsg("Timeout connecting to server port %hu", port); + sclose(serverfd); + return CURL_SOCKET_BAD; + } + } + } +error: logmsg("Error connecting to server port %hu: (%d) %s", port, error, sstrerror(error)); sclose(serverfd); return CURL_SOCKET_BAD; } - +success: logmsg("connected fine to %s%s%s:%hu, now tunnel", op_br, ipaddr, cl_br, port); + if(0 != curlx_nonblock(serverfd, FALSE)) { + error = SOCKERRNO; + logmsg("curlx_nonblock(FALSE) failed with error: (%d) %s", + error, sstrerror(error)); + sclose(serverfd); + return CURL_SOCKET_BAD; + } + return serverfd; } diff --git a/tests/servers.pm b/tests/servers.pm index 887cf271a..433f3f234 100644 --- a/tests/servers.pm +++ b/tests/servers.pm @@ -445,6 +445,7 @@ sub protoport { # sub stopserver { my ($server, $pidlist) = @_; + my $ipvnum = 4; # # kill sockfilter processes for pingpong relative server @@ -452,7 +453,7 @@ sub stopserver { if($server =~ /^(ftp|imap|pop3|smtp)s?(\d*)(-ipv6|)$/) { my $proto = $1; my $idnum = ($2 && ($2 > 1)) ? $2 : 1; - my $ipvnum = ($3 && ($3 =~ /6$/)) ? 6 : 4; + $ipvnum = ($3 && ($3 =~ /6$/)) ? 6 : 4; killsockfilters("$LOGDIR/$PIDDIR", $proto, $ipvnum, $idnum, $verbose); } # @@ -503,6 +504,23 @@ sub stopserver { my $pidfile = $serverpidfile{$server}; unlink($pidfile) if(-f $pidfile); } + # + # cleanup server lock files + # + foreach my $server (@killservers) { + # servers seem to produce (some of) these lock files + my @lockfiles = ( + "$LOGDIR/$LOCKDIR/$server.lock", + "$LOGDIR/$LOCKDIR/$server-IPv$ipvnum.lock", + "$LOGDIR/$LOCKDIR/sws-".uc($server)."-IPv$ipvnum.lock" + ); + foreach my $lockfile (@lockfiles) { + if(-f $lockfile) { + unlink($lockfile); + logmsg "RUN: kill $server, cleaned up $lockfile\n" if ($verbose); + } + } + } return $result; } @@ -2326,7 +2344,7 @@ sub startservers { ($what eq "ftp") || ($what eq "imap") || ($what eq "smtp")) { - if($torture && $run{$what} && + if($run{$what} && !responsive_pingpong_server($what, "", $verbose)) { if(stopserver($what)) { return ("failed stopping unresponsive ".uc($what)." server", 3); @@ -2342,7 +2360,7 @@ sub startservers { } } elsif($what eq "ftp-ipv6") { - if($torture && $run{'ftp-ipv6'} && + if($run{'ftp-ipv6'} && !responsive_pingpong_server("ftp", "", $verbose, "ipv6")) { if(stopserver('ftp-ipv6')) { return ("failed stopping unresponsive FTP-IPv6 server", 3); @@ -2359,7 +2377,7 @@ sub startservers { } } elsif($what eq "gopher") { - if($torture && $run{'gopher'} && + if($run{'gopher'} && !responsive_http_server("gopher", $verbose, 0, protoport("gopher"))) { if(stopserver('gopher')) { @@ -2378,7 +2396,7 @@ sub startservers { } } elsif($what eq "gopher-ipv6") { - if($torture && $run{'gopher-ipv6'} && + if($run{'gopher-ipv6'} && !responsive_http_server("gopher", $verbose, "ipv6", protoport("gopher"))) { if(stopserver('gopher-ipv6')) { @@ -2439,7 +2457,7 @@ sub startservers { } } elsif($what eq "http-proxy") { - if($torture && $run{'http-proxy'} && + if($run{'http-proxy'} && !responsive_http_server("http", $verbose, "proxy", protoport("httpproxy"))) { if(stopserver('http-proxy')) { @@ -2458,7 +2476,7 @@ sub startservers { } } elsif($what eq "http-ipv6") { - if($torture && $run{'http-ipv6'} && + if($run{'http-ipv6'} && !responsive_http_server("http", $verbose, "ipv6", protoport("http6"))) { if(stopserver('http-ipv6')) { @@ -2477,7 +2495,7 @@ sub startservers { } } elsif($what eq "rtsp") { - if($torture && $run{'rtsp'} && + if($run{'rtsp'} && !responsive_rtsp_server($verbose)) { if(stopserver('rtsp')) { return ("failed stopping unresponsive RTSP server", 3); @@ -2493,7 +2511,7 @@ sub startservers { } } elsif($what eq "rtsp-ipv6") { - if($torture && $run{'rtsp-ipv6'} && + if($run{'rtsp-ipv6'} && !responsive_rtsp_server($verbose, "ipv6")) { if(stopserver('rtsp-ipv6')) { return ("failed stopping unresponsive RTSP-IPv6 server", 3); @@ -2521,7 +2539,7 @@ sub startservers { return ("failed stopping $what server with different cert", 3); } } - if($torture && $run{$cproto} && + if($run{$cproto} && !responsive_pingpong_server($cproto, "", $verbose)) { if(stopserver($cproto)) { return ("failed stopping unresponsive $cproto server", 3); @@ -2561,7 +2579,14 @@ sub startservers { return ("failed stopping HTTPS server with different cert", 3); } } - if($torture && $run{'http'} && + if($run{'https'} && + !responsive_http_server("https", $verbose, 0, + protoport('https'))) { + if(stopserver('https')) { + return ("failed stopping unresponsive HTTPS server", 3); + } + } + if($run{'http'} && !responsive_http_server("http", $verbose, 0, protoport('http'))) { if(stopserver('http')) { @@ -2599,7 +2624,7 @@ sub startservers { return ("failed stopping GOPHERS server with different cert", 3); } } - if($torture && $run{'gopher'} && + if($run{'gopher'} && !responsive_http_server("gopher", $verbose, 0, protoport('gopher'))) { if(stopserver('gopher')) { @@ -2668,7 +2693,7 @@ sub startservers { # for now, we can't run http TLS-EXT tests without gnutls-serv return ("no gnutls-serv (with SRP support)", 4); } - if($torture && $run{'httptls'} && + if($run{'httptls'} && !responsive_httptls_server($verbose, "IPv4")) { if(stopserver('httptls')) { return ("failed stopping unresponsive HTTPTLS server", 3); @@ -2690,7 +2715,7 @@ sub startservers { # for now, we can't run http TLS-EXT tests without gnutls-serv return ("no gnutls-serv", 4); } - if($torture && $run{'httptls-ipv6'} && + if($run{'httptls-ipv6'} && !responsive_httptls_server($verbose, "ipv6")) { if(stopserver('httptls-ipv6')) { return ("failed stopping unresponsive HTTPTLS-IPv6 server", 3); @@ -2708,7 +2733,7 @@ sub startservers { } } elsif($what eq "tftp") { - if($torture && $run{'tftp'} && + if($run{'tftp'} && !responsive_tftp_server("", $verbose)) { if(stopserver('tftp')) { return ("failed stopping unresponsive TFTP server", 3); @@ -2725,7 +2750,7 @@ sub startservers { } } elsif($what eq "tftp-ipv6") { - if($torture && $run{'tftp-ipv6'} && + if($run{'tftp-ipv6'} && !responsive_tftp_server("", $verbose, "ipv6")) { if(stopserver('tftp-ipv6')) { return ("failed stopping unresponsive TFTP-IPv6 server", 3); @@ -2782,7 +2807,7 @@ sub startservers { } } elsif($what eq "http-unix") { - if($torture && $run{'http-unix'} && + if($run{'http-unix'} && !responsive_http_server("http", $verbose, "unix", $HTTPUNIXPATH)) { if(stopserver('http-unix')) { return ("failed stopping unresponsive HTTP-unix server", 3);