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

Reply via email to