From 5e65e080ad4d57eb3fcb7b53980cf6a4d1e8ae19 Mon Sep 17 00:00:00 2001 From: Gurucharan Shetty Date: Tue, 7 Apr 2015 17:17:39 -0700 Subject: [PATCH] tests: Avoid Windows unit tests from hanging. It has been observed that sometimes Windows unit tests hang. This happens when a daemon is started but does not get terminated when the test ends. In one particular case, OVS_VSWITCHD_STOP is called which inturn calls 'ovs-appctl exit'. This causes ovs-vswitchd's atexit handler to cleanup the pidfiles. After this, the pthread destructurs get called and a deadlock happens in there. This results in the daemons not getting force killed resulting in the tests hanging because the cleanup file tries to run the command "kill `cat ovs-vswitchd.pid`" and ovs-vswitchd.pid no longer exists. With this commit, we write the pid value of the daemons in the cleanup file (instead of asking it to 'cat' the value later from the pidfile). This way, even if the pidfiles get deleted, we can still kill the daemons. This commit also changes the way daemons are force killed in Windows. It was observed that 'taskkill //F ' failed to kill a deadlocked daemon running its pthread destructor. But tskill succeeds. Signed-off-by: Gurucharan Shetty (ON_EXIT_UNQUOTED macro provided by Ben.) Co-authored-by: Ben Pfaff Signed-off-by: Ben Pfaff --- tests/ofproto-macros.at | 3 ++- tests/ovs-macros.at | 35 ++++++++++++++++++++++++++--------- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at index 93d8a7751..fd915ef28 100644 --- a/tests/ofproto-macros.at +++ b/tests/ofproto-macros.at @@ -49,7 +49,6 @@ m4_define([_OVS_VSWITCHD_START], OVS_LOGDIR=`pwd`; export OVS_LOGDIR OVS_DBDIR=`pwd`; export OVS_DBDIR OVS_SYSCONFDIR=`pwd`; export OVS_SYSCONFDIR - ON_EXIT([kill `cat ovsdb-server.pid ovs-vswitchd.pid`]) dnl Create database. touch .conf.db.~lock~ @@ -57,6 +56,7 @@ m4_define([_OVS_VSWITCHD_START], dnl Start ovsdb-server. AT_CHECK([ovsdb-server --detach --no-chdir --pidfile --log-file --remote=punix:$OVS_RUNDIR/db.sock], [0], [], [stderr]) + ON_EXIT_UNQUOTED([kill `cat ovsdb-server.pid`]) AT_CHECK([[sed < stderr ' /vlog|INFO|opened log file/d /ovsdb_server|INFO|ovsdb-server (Open vSwitch)/d']]) @@ -68,6 +68,7 @@ m4_define([_OVS_VSWITCHD_START], dnl Start ovs-vswitchd. AT_CHECK([ovs-vswitchd $1 --detach --no-chdir --pidfile --log-file -vvconn -vofproto_dpif], [0], [], [stderr]) AT_CAPTURE_FILE([ovs-vswitchd.log]) + ON_EXIT_UNQUOTED([kill `cat ovs-vswitchd.pid`]) AT_CHECK([[sed < stderr ' /ovs_numa|INFO|Discovered /d /vlog|INFO|opened log file/d diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at index 14edba373..c583c3d40 100644 --- a/tests/ovs-macros.at +++ b/tests/ovs-macros.at @@ -55,12 +55,16 @@ if test "$IS_WIN32" = "yes"; then -[1-9]*) shift for i in $*; do - taskkill //F //PID $i >/dev/null + if tasklist //fi "PID eq $i" | grep $i >/dev/null; then + tskill $i + fi done ;; [1-9][0-9]*) for i in $*; do - taskkill //F //PID $i >/dev/null + if tasklist //fi "PID eq $i" | grep $i >/dev/null; then + tskill $i + fi done ;; esac @@ -86,15 +90,28 @@ m4_define([OVS_APP_EXIT_AND_WAIT], [ovs-appctl -t $1 exit OVS_WAIT_WHILE([test -e $1.pid])]) +m4_define([ON_EXIT__], [trap '. ./cleanup' 0; cat - cleanup << $2 > __cleanup +$1 +EOF +mv __cleanup cleanup +]) + dnl ON_EXIT([COMMANDS]) +dnl ON_EXIT_UNQUOTED([COMMANDS]) dnl -dnl Adds the shell COMMANDS to a collection executed when the current test +dnl Add the shell COMMANDS to a collection executed when the current test dnl completes, as a cleanup action. (The most common use is to kill a dnl daemon started by the test. This is important to prevent tests that dnl start daemons from hanging at exit.) -dnl The commands will be added will be tht first one to excute. -m4_define([ON_EXIT], [trap '. ./cleanup' 0; cat - cleanup << 'EOF' > __cleanup -$1 -EOF -mv __cleanup cleanup -]) +dnl +dnl The only difference between ON_EXIT and ON_EXIT_UNQUOTED is that only the +dnl latter performs shell variable (e.g. $var) substitution, command +dnl substitution (e.g. `command`), and backslash escaping (e.g. \\ becomes \) +dnl in COMMANDS at the time that ON_EXIT_UNQUOTED is encountered. ON_EXIT, +dnl in contrast, copies the literal COMMANDS and only executes shell expansion +dnl at cleanup time. +dnl +dnl Cleanup commands are executed in the reverse order of execution of +dnl these macros. +m4_define([ON_EXIT], [ON_EXIT__([$1], ['EOF'])]) +m4_define([ON_EXIT_UNQUOTED], [ON_EXIT__([$1], [EOF])]) -- 2.20.1