Message ID | 20200212131423.27742-1-msc@linux.ibm.com |
---|---|
State | New |
Headers | show |
Series | sunrpc: Properly clean up if tst-udp-timeout fails | expand |
* Matheus Castanho: > Hi Florian, > > I have experimented with the options you listed above, but will probably > leave them for a future patch, as a generic solution will likely require more > careful consideration. > > For the moment I dropped the extra TEST_VERIFY macro and fixed this single > test using the atexit() approach you pointed. I think it is preferable to > removing the TEST_VERIFY_EXIT calls, as without them we would have to run > the entire test (17-25 secs) even if an error ocurred in the beginning. > Also, I don't think we need to add more complexity to this test (e.g. > moving the client to a subprocess), as it already has a lot going on, so > I opted for this simpler approach. > > The updated patch is below. Thanks, I like this approach. Some nits below. > The macro TEST_VERIFY_EXIT is used several times on > sunrpc/tst-udp-timeout to exit the test if a condition evaluates to > false. The side effect is that the code to terminate the RPC server > process is not executed when the program calls exit(), so that > sub-process stays alive. > > This commit registers a clean up function with atexit() to kill the > server process before exiting the main program. GNU style doesn't use () after function names, so this should just be atexit. > --- > sunrpc/tst-udp-timeout.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/sunrpc/tst-udp-timeout.c b/sunrpc/tst-udp-timeout.c > index 8d45365b23..de2fc3bf8c 100644 > --- a/sunrpc/tst-udp-timeout.c > +++ b/sunrpc/tst-udp-timeout.c > @@ -29,6 +29,9 @@ > #include <sys/socket.h> > #include <time.h> > #include <unistd.h> > +#include <stdlib.h> > + > +pid_t pid; Please make this variable static. It should also be called server_pid, to make its purpose clear. > +/* Function to be called before exit() to make sure the > + * server process is properly killed. */ Style nit: exit instead of exit(), comments should not be indented with *, like this: /* Function to be called before exit to make sure the server process is properly killed. */
diff --git a/sunrpc/tst-udp-timeout.c b/sunrpc/tst-udp-timeout.c index 8d45365b23..de2fc3bf8c 100644 --- a/sunrpc/tst-udp-timeout.c +++ b/sunrpc/tst-udp-timeout.c @@ -29,6 +29,9 @@ #include <sys/socket.h> #include <time.h> #include <unistd.h> +#include <stdlib.h> + +pid_t pid; /* Test data serialization and deserialization. */ @@ -177,6 +180,14 @@ server_dispatch (struct svc_req *request, SVCXPRT *transport) } } +/* Function to be called before exit() to make sure the + * server process is properly killed. */ +static void +kill_server (void) +{ + kill(pid, SIGTERM); +} + /* Implementation of the test client. */ static struct test_response @@ -381,12 +392,13 @@ do_test (void) TEST_VERIFY_EXIT (transport != NULL); TEST_VERIFY (svc_register (transport, PROGNUM, VERSNUM, server_dispatch, 0)); - pid_t pid = xfork (); + pid = xfork (); if (pid == 0) { svc_run (); FAIL_EXIT1 ("supposed to be unreachable"); } + atexit(kill_server); test_udp_server (transport->xp_port); int status;