On Tue, Apr 07, 2015 at 06:06:40PM -0700, Gurucharan Shetty wrote: > 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. > > This commit stores the contents of pidfiles in different files. > This way, even if the pidfiles get deleted for any reason, 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 <gshe...@nicira.com>
This seems reasonable to me. I think that we might be able to implement it a little more nicely. What if we introduce an ON_EXIT_UNQUOTED, analogous to AT_CHECK_UNQUOTED, and use it here in OVS_VSWITCHD_START instead of making a on-disk file copy? diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at index 14edba3..d06e200 100644 --- a/tests/ovs-macros.at +++ b/tests/ovs-macros.at @@ -86,15 +86,29 @@ 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])]) + _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev