From patchwork Wed Feb 5 21:05:17 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matheus Castanho X-Patchwork-Id: 1234010 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=sourceware.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=libc-alpha-return-109203-incoming=patchwork.ozlabs.org@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=linux.ibm.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha1 header.s=default header.b=VnYc5FHX; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 48CYxS42qsz9sPK for ; Thu, 6 Feb 2020 08:05:31 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:subject:date:message-id:mime-version :content-transfer-encoding; q=dns; s=default; b=VrX4Ykh3y6TpwWnV B3Z8rr2HGtxLUJ2UOq8Wvko1y+lllZ7GzK4fPSbSJbnsKrTNjgtIv1xIBmSY9fid y2i8g0uLoqrWw3DMCLUdiMS17KqM1xPA5ONbH1W1/GCLmaC3jsFlwW5XWCI+3/Lk JC2i5kdnbc1vPWLAJxtufCC9/I0= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:subject:date:message-id:mime-version :content-transfer-encoding; s=default; bh=z9cyfC1uxrC9DOp2U8nfK/ zkEiY=; b=VnYc5FHXJlCpNUFNLncX2ODXY80muuaYrgeyl98J7tE/1JtQM4KiXU ZU8Pvv0BuyPdpar2R6H72EJlFXdcylhKEX+skJsW2jpTjeHSrqmUvCb6hIV1OoEp SpoQ5DE8vXljfZbs3i/CprWFoBwPIeZhIijrG1EZTFET/Fmi3sW/U= Received: (qmail 39059 invoked by alias); 5 Feb 2020 21:05:25 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 39048 invoked by uid 89); 5 Feb 2020 21:05:24 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-19.7 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.1 spammy=sock, subprocess, orphan X-HELO: mx0a-001b2d01.pphosted.com From: Matheus Castanho To: libc-alpha@sourceware.org Subject: [RFC] sunrpc: Properly cleanup if tst-udp-timeout fails Date: Wed, 5 Feb 2020 18:05:17 -0300 Message-Id: <20200205210517.21850-1-msc@linux.ibm.com> MIME-Version: 1.0 Hi, I recently noted some orphan processes left on a host after failed sunrpc/tst-udp-timeout.c runs. More info on the commit message. Even though the test infra has support for a cleanup_function, it is only called when the test times out. The same happens for the code to kill the whole process group (both on the signal handler in support/support_test_main.c). So I ended up adding a cleanup function to be called right before exit() to terminate the child. I also considered using a function with __attribute__((destructor)) to perform the cleanup, but adding a new TEST_VERIFY_* macro seemed to be more reusable by other tests if needed. Is this the best way to handle this issue? ----8<---- 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 adds a new TEST_VERIFY_CLEANUP macro to allow specifying a cleanup function to be called if the check fails. TEST_VERIFY_EXIT calls in tst-udp-timeout that may leave the orphan process behind are replaced by this macro so a cleanup function is run before exiting to guarantee the server is properly terminated in case of a test failure. --- sunrpc/tst-udp-timeout.c | 29 ++++++++++++++++++++--------- support/check.h | 12 ++++++++++++ 2 files changed, 32 insertions(+), 9 deletions(-) -- 2.21.1 diff --git a/sunrpc/tst-udp-timeout.c b/sunrpc/tst-udp-timeout.c index 8d45365b23..a936afd56c 100644 --- a/sunrpc/tst-udp-timeout.c +++ b/sunrpc/tst-udp-timeout.c @@ -30,6 +30,8 @@ #include #include +pid_t pid; + /* Test data serialization and deserialization. */ struct test_query @@ -66,6 +68,15 @@ xdr_test_response (XDR *xdrs, void *data, ...) && xdr_uint32_t (xdrs, &p->sum); } +/* Cleanup function to be called in case of failure. */ +static void +cleanup (void) +{ + /* Kill the child process running the RPC server. */ + kill(pid, SIGTERM); + exit(1); +} + /* Implementation of the test server. */ enum @@ -188,11 +199,11 @@ test_call (CLIENT *clnt, int proc, struct test_query query, proc, (unsigned long) timeout.tv_sec, (unsigned long) timeout.tv_usec); struct test_response response; - TEST_VERIFY_EXIT (clnt_call (clnt, proc, + TEST_VERIFY_CLEANUP (clnt_call (clnt, proc, xdr_test_query, (void *) &query, xdr_test_response, (void *) &response, timeout) - == RPC_SUCCESS); + == RPC_SUCCESS, cleanup ()); return response; } @@ -218,11 +229,11 @@ test_call_flush (CLIENT *clnt) requested via the timeout_ms field. */ if (test_verbose) printf ("info: flushing pending queries\n"); - TEST_VERIFY_EXIT (clnt_call (clnt, PROC_RESET_SEQ, + TEST_VERIFY_CLEANUP (clnt_call (clnt, PROC_RESET_SEQ, (xdrproc_t) xdr_void, NULL, (xdrproc_t) xdr_void, NULL, ((struct timeval) { 5, 0 })) - == RPC_SUCCESS); + == RPC_SUCCESS, cleanup ()); } /* Return the number seconds since an arbitrary point in time. */ @@ -236,7 +247,7 @@ get_ticks (void) } { struct timeval tv; - TEST_VERIFY_EXIT (gettimeofday (&tv, NULL) == 0); + TEST_VERIFY_CLEANUP (gettimeofday (&tv, NULL) == 0, cleanup ()); return tv.tv_sec + tv.tv_usec * 1e-6; } } @@ -259,7 +270,7 @@ test_udp_server (int port) CLIENT *clnt = clntudp_create (&sin, PROGNUM, VERSNUM, (struct timeval) { 1, 500 * 1000 }, &sock); - TEST_VERIFY_EXIT (clnt != NULL); + TEST_VERIFY_CLEANUP (clnt != NULL, cleanup ()); /* Basic call/response test. */ struct test_response response = test_call @@ -363,11 +374,11 @@ test_udp_server (int port) test_call_flush (clnt); } - TEST_VERIFY_EXIT (clnt_call (clnt, PROC_EXIT, + TEST_VERIFY_CLEANUP (clnt_call (clnt, PROC_EXIT, (xdrproc_t) xdr_void, NULL, (xdrproc_t) xdr_void, NULL, ((struct timeval) { 5, 0 })) - == RPC_SUCCESS); + == RPC_SUCCESS, cleanup ()); clnt_destroy (clnt); } @@ -381,7 +392,7 @@ 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 (); diff --git a/support/check.h b/support/check.h index 77d1d1e14d..5f090dae3e 100644 --- a/support/check.h +++ b/support/check.h @@ -64,6 +64,18 @@ __BEGIN_DECLS (1, __FILE__, __LINE__, #expr); \ }) +/* Record a test failure and run CLEANUP if EXPR evaluates to false. */ +#define TEST_VERIFY_CLEANUP(expr, cleanup) \ + ({ \ + if (expr) \ + ; \ + else \ + { \ + support_test_verify_impl (__FILE__, __LINE__, #expr); \ + cleanup; \ + } \ + }) + int support_print_failure_impl (const char *file, int line,