On Tue, Apr 14, 2015 at 9:02 PM, Ben Pfaff <b...@nicira.com> wrote: > 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?
I am sorry for the delayed response. I somehow completely missed your feedback. The change that you suggest is very clever (I did not know the difference between quoted and unquoted word after the HERE document till today.) . I will incorporate it as part of v2. > > 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
