From: Stanislav K. <sta...@or...> - 2013-10-29 07:55:49
|
I came accross the following rare issue: setpgid03 1 TPASS : setpgid SUCCESS to set errno to EPERM setpgid03 2 TFAIL : setpgid FAILED, expect EACCES got 1 I suppose It may happen if the test host is under stress. To prevent this It would be better to use SIGUSR1 for parent-child synchronisations. Also fixed error passing from children and performed a minor cleanup. Signed-off-by: Stanislav Kholmanskikh <sta...@or...> --- testcases/kernel/syscalls/setpgid/setpgid03.c | 200 ++++++++++++------------- 1 files changed, 98 insertions(+), 102 deletions(-) diff --git a/testcases/kernel/syscalls/setpgid/setpgid03.c b/testcases/kernel/syscalls/setpgid/setpgid03.c index 4429fe4..17fd16f 100644 --- a/testcases/kernel/syscalls/setpgid/setpgid03.c +++ b/testcases/kernel/syscalls/setpgid/setpgid03.c @@ -1,34 +1,23 @@ /* + * Copyright (c) International Business Machines Corp., 2001 * - * Copyright (c) International Business Machines Corp., 2001 + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See + * the GNU General Public License for more details. * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See - * the GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA */ /* - * NAME - * setpgid03.c - * - * DESCRIPTION - * Test to check the error and trivial conditions in setpgid system call - * - * USAGE - * setuid03 - * - * RESTRICTIONS - * This test is not completely written in the LTP format - PLEASE FIX! + * Test to check the error and trivial conditions in setpgid system call */ #define DEBUG 0 @@ -47,16 +36,16 @@ char *TCID = "setpgid03"; int TST_TOTAL = 1; -void do_child(void); -void setup(void); -void cleanup(void); +static void do_child(void); +static void setup(void); +static void cleanup(void); int main(int ac, char **av) { int pid; - int rval, fail = 0; - int ret, status; - int exno = 0; + int rval; + int status; + sigset_t sigusr_set; int lc; char *msg; @@ -68,14 +57,13 @@ int main(int ac, char **av) maybe_run_child(&do_child, ""); #endif - /* - * perform global setup for the test - */ setup(); + sigemptyset(&sigusr_set); + sigaddset(&sigusr_set, SIGUSR1); + for (lc = 0; TEST_LOOPING(lc); lc++) { - /* reset tst_count in case we are looping */ tst_count = 0; //test1: @@ -84,7 +72,7 @@ int main(int ac, char **av) tst_brkm(TBROK, cleanup, "fork() failed"); } - if (pid == 0) { /* child */ + if (pid == 0) { #ifdef UCLINUX if (self_exec(av[0], "") < 0) { tst_brkm(TBROK, cleanup, "self_exec failed"); @@ -92,35 +80,33 @@ int main(int ac, char **av) #else do_child(); #endif - } else { /* parent */ - sleep(1); - rval = setpgid(pid, getppid()); - if (errno == EPERM) { - tst_resm(TPASS, "setpgid SUCCESS to set " - "errno to EPERM"); - } else { - tst_resm(TFAIL, "setpgid FAILED, " - "expect %d, return %d", EPERM, errno); - fail = 1; - } - sleep(1); - if ((ret = wait(&status)) > 0) { - if (DEBUG) - tst_resm(TINFO, "Test {%d} exited " - "status 0x%0x", ret, status); - - if (status != 0) { - fail = 1; - } - } } - if (DEBUG) { - if (fail || exno) { - tst_resm(TINFO, "Test test 1: FAILED"); - } else { - tst_resm(TINFO, "Test test 1: PASSED"); - } + + if (sigwaitinfo(&sigusr_set, NULL) < 0) + tst_brkm(TFAIL | TERRNO, cleanup, + "sigwaitinfo() failed"); + + rval = setpgid(pid, getppid()); + if (errno == EPERM) { + tst_resm(TPASS, + "setpgid SUCCESS to set errno to EPERM"); + } else { + tst_resm(TFAIL, + "setpgid FAILED, expect %d, return %d", + EPERM, errno); + } + + if (wait(&status) < 0) + tst_resm(TFAIL | TERRNO, "wait() failed"); + + if (!(WIFEXITED(status)) || (WEXITSTATUS(status) != 0)) { + if (DEBUG) + tst_resm(TINFO, "child exited with status %d", + WEXITSTATUS(status)); + + tst_resm(TFAIL, "child failed"); } + //test2: /* * Value of pid matches the pid of the child process and @@ -131,39 +117,59 @@ int main(int ac, char **av) } if (pid == 0) { - if (execlp("sleep", "sleep", "3", NULL) < 0) { - perror("exec failed"); + char buf[10]; + int ret; + + ret = snprintf(buf, 10, "%d", getppid()); + if ((ret >= 10) || (ret <= 0)) { + printf("snprintf() failed\n"); + + exit(126); } + + const char *args[4 + 1]; + args[0] = "kill"; + args[1] = "-s"; + args[2] = "SIGUSR1"; + args[3] = buf; + args[4] = NULL; + + if (execvp(args[0], (char *const *)args) < 0) + perror("exec failed\n"); + exit(127); + } + + if (sigwaitinfo(&sigusr_set, NULL) < 0) + tst_brkm(TFAIL | TERRNO, cleanup, + "sigwaitinfo() failed"); + + rval = setpgid(pid, getppid()); + if (errno == EACCES) { + tst_resm(TPASS, + "setpgid SUCCEEDED to set errno to EACCES"); } else { - sleep(1); - rval = setpgid(pid, getppid()); - if (errno == EACCES) { - tst_resm(TPASS, "setpgid SUCCEEDED to set " - "errno to EACCES"); - } else { - tst_resm(TFAIL, "setpgid FAILED, expect EACCES " - "got %d", errno); - } - if ((ret = wait(&status)) > 0) { - if (DEBUG) - tst_resm(TINFO, "Test {%d} exited " - "status 0x%0x", ret, status); - if (status != 0) { - fail = 1; - } - } + tst_resm(TFAIL, + "setpgid FAILED, expect EACCES got %d", errno); + } + + if (wait(&status) < 0) + tst_resm(TFAIL | TERRNO, "wait() failed"); + + if ((!WIFEXITED(status)) || (WEXITSTATUS(status) != 0)) { + if (DEBUG) + tst_resm(TINFO, "child exited with status %d", + WEXITSTATUS(status)); + + tst_resm(TFAIL, "child failed"); } } + cleanup(); tst_exit(); - } -/* - * do_child() - */ -void do_child() +static void do_child(void) { int exno = 0; @@ -171,15 +177,15 @@ void do_child() tst_resm(TFAIL, "setsid failed, errno :%d", errno); exno = 1; } + + kill(getppid(), SIGUSR1); + sleep(2); + exit(exno); } -/* - * setup() - * performs all ONE TIME setup for this test - */ -void setup(void) +static void setup(void) { tst_sig(FORK, DEF_HANDLER, cleanup); @@ -189,17 +195,7 @@ void setup(void) TEST_PAUSE; } -/* - * cleanup() - * performs all the ONE TIME cleanup for this test at completion - * or premature exit - */ -void cleanup(void) +static void cleanup(void) { - /* - * print timing status if that option was specified - * print errno log if that option was specified - */ TEST_CLEANUP; - } -- 1.7.1 |
From: Jan S. <jst...@re...> - 2013-10-29 10:30:43
|
Hi, it looks race-y after patch too. For example, let assume child runs first: diff --git a/testcases/kernel/syscalls/setpgid/setpgid03.c b/testcases/kernel/syscalls/setpgid/setpgid03.c index 17fd16f..91f9940 100644 --- a/testcases/kernel/syscalls/setpgid/setpgid03.c +++ b/testcases/kernel/syscalls/setpgid/setpgid03.c @@ -82,6 +82,7 @@ int main(int ac, char **av) #endif } + sleep(1); if (sigwaitinfo(&sigusr_set, NULL) < 0) tst_brkm(TFAIL | TERRNO, cleanup, "sigwaitinfo() failed"); # ./setpgid03 setpgid03 1 TBROK : unexpected signal 10 received (pid = 6763). setpgid03 2 TBROK : Remaining cases broken It might be easier to use "checkpoint" system, see include/tst_checkpoint.h, and lib/tests/tst_checkpoint_* for examples. Regards, Jan ----- Original Message ----- > From: "Stanislav Kholmanskikh" <sta...@or...> > To: ltp...@li... > Cc: "vasily isaenko" <vas...@or...> > Sent: Tuesday, 29 October, 2013 8:55:32 AM > Subject: [LTP] [PATCH] setpgid03: use SIGUSR1 instead of sleep > > I came accross the following rare issue: > > setpgid03 1 TPASS : setpgid SUCCESS to set errno to EPERM > setpgid03 2 TFAIL : setpgid FAILED, expect EACCES got 1 > > I suppose It may happen if the test host is under stress. > To prevent this It would be better to use SIGUSR1 for parent-child > synchronisations. > > Also fixed error passing from children and performed a minor cleanup. > > Signed-off-by: Stanislav Kholmanskikh <sta...@or...> > --- > testcases/kernel/syscalls/setpgid/setpgid03.c | 200 > ++++++++++++------------- > 1 files changed, 98 insertions(+), 102 deletions(-) > > diff --git a/testcases/kernel/syscalls/setpgid/setpgid03.c > b/testcases/kernel/syscalls/setpgid/setpgid03.c > index 4429fe4..17fd16f 100644 > --- a/testcases/kernel/syscalls/setpgid/setpgid03.c > +++ b/testcases/kernel/syscalls/setpgid/setpgid03.c > @@ -1,34 +1,23 @@ > /* > + * Copyright (c) International Business Machines Corp., 2001 > * > - * Copyright (c) International Business Machines Corp., 2001 > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > * > - * This program is free software; you can redistribute it and/or modify > - * it under the terms of the GNU General Public License as published by > - * the Free Software Foundation; either version 2 of the License, or > - * (at your option) any later version. > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See > + * the GNU General Public License for more details. > * > - * This program is distributed in the hope that it will be useful, > - * but WITHOUT ANY WARRANTY; without even the implied warranty of > - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See > - * the GNU General Public License for more details. > - * > - * You should have received a copy of the GNU General Public License > - * along with this program; if not, write to the Free Software > - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA > 02110-1301 USA > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 > USA > */ > > /* > - * NAME > - * setpgid03.c > - * > - * DESCRIPTION > - * Test to check the error and trivial conditions in setpgid system call > - * > - * USAGE > - * setuid03 > - * > - * RESTRICTIONS > - * This test is not completely written in the LTP format - PLEASE FIX! > + * Test to check the error and trivial conditions in setpgid system call > */ > > #define DEBUG 0 > @@ -47,16 +36,16 @@ > char *TCID = "setpgid03"; > int TST_TOTAL = 1; > > -void do_child(void); > -void setup(void); > -void cleanup(void); > +static void do_child(void); > +static void setup(void); > +static void cleanup(void); > > int main(int ac, char **av) > { > int pid; > - int rval, fail = 0; > - int ret, status; > - int exno = 0; > + int rval; > + int status; > + sigset_t sigusr_set; > > int lc; > char *msg; > @@ -68,14 +57,13 @@ int main(int ac, char **av) > maybe_run_child(&do_child, ""); > #endif > > - /* > - * perform global setup for the test > - */ > setup(); > > + sigemptyset(&sigusr_set); > + sigaddset(&sigusr_set, SIGUSR1); > + > for (lc = 0; TEST_LOOPING(lc); lc++) { > > - /* reset tst_count in case we are looping */ > tst_count = 0; > > //test1: > @@ -84,7 +72,7 @@ int main(int ac, char **av) > tst_brkm(TBROK, cleanup, "fork() failed"); > } > > - if (pid == 0) { /* child */ > + if (pid == 0) { > #ifdef UCLINUX > if (self_exec(av[0], "") < 0) { > tst_brkm(TBROK, cleanup, "self_exec failed"); > @@ -92,35 +80,33 @@ int main(int ac, char **av) > #else > do_child(); > #endif > - } else { /* parent */ > - sleep(1); > - rval = setpgid(pid, getppid()); > - if (errno == EPERM) { > - tst_resm(TPASS, "setpgid SUCCESS to set " > - "errno to EPERM"); > - } else { > - tst_resm(TFAIL, "setpgid FAILED, " > - "expect %d, return %d", EPERM, errno); > - fail = 1; > - } > - sleep(1); > - if ((ret = wait(&status)) > 0) { > - if (DEBUG) > - tst_resm(TINFO, "Test {%d} exited " > - "status 0x%0x", ret, status); > - > - if (status != 0) { > - fail = 1; > - } > - } > } > - if (DEBUG) { > - if (fail || exno) { > - tst_resm(TINFO, "Test test 1: FAILED"); > - } else { > - tst_resm(TINFO, "Test test 1: PASSED"); > - } > + > + if (sigwaitinfo(&sigusr_set, NULL) < 0) > + tst_brkm(TFAIL | TERRNO, cleanup, > + "sigwaitinfo() failed"); > + > + rval = setpgid(pid, getppid()); > + if (errno == EPERM) { > + tst_resm(TPASS, > + "setpgid SUCCESS to set errno to EPERM"); > + } else { > + tst_resm(TFAIL, > + "setpgid FAILED, expect %d, return %d", > + EPERM, errno); > + } > + > + if (wait(&status) < 0) > + tst_resm(TFAIL | TERRNO, "wait() failed"); > + > + if (!(WIFEXITED(status)) || (WEXITSTATUS(status) != 0)) { > + if (DEBUG) > + tst_resm(TINFO, "child exited with status %d", > + WEXITSTATUS(status)); > + > + tst_resm(TFAIL, "child failed"); > } > + > //test2: > /* > * Value of pid matches the pid of the child process and > @@ -131,39 +117,59 @@ int main(int ac, char **av) > > } > if (pid == 0) { > - if (execlp("sleep", "sleep", "3", NULL) < 0) { > - perror("exec failed"); > + char buf[10]; > + int ret; > + > + ret = snprintf(buf, 10, "%d", getppid()); > + if ((ret >= 10) || (ret <= 0)) { > + printf("snprintf() failed\n"); > + > + exit(126); > } > + > + const char *args[4 + 1]; > + args[0] = "kill"; > + args[1] = "-s"; > + args[2] = "SIGUSR1"; > + args[3] = buf; > + args[4] = NULL; > + > + if (execvp(args[0], (char *const *)args) < 0) > + perror("exec failed\n"); > + > exit(127); > + } > + > + if (sigwaitinfo(&sigusr_set, NULL) < 0) > + tst_brkm(TFAIL | TERRNO, cleanup, > + "sigwaitinfo() failed"); > + > + rval = setpgid(pid, getppid()); > + if (errno == EACCES) { > + tst_resm(TPASS, > + "setpgid SUCCEEDED to set errno to EACCES"); > } else { > - sleep(1); > - rval = setpgid(pid, getppid()); > - if (errno == EACCES) { > - tst_resm(TPASS, "setpgid SUCCEEDED to set " > - "errno to EACCES"); > - } else { > - tst_resm(TFAIL, "setpgid FAILED, expect EACCES " > - "got %d", errno); > - } > - if ((ret = wait(&status)) > 0) { > - if (DEBUG) > - tst_resm(TINFO, "Test {%d} exited " > - "status 0x%0x", ret, status); > - if (status != 0) { > - fail = 1; > - } > - } > + tst_resm(TFAIL, > + "setpgid FAILED, expect EACCES got %d", errno); > + } > + > + if (wait(&status) < 0) > + tst_resm(TFAIL | TERRNO, "wait() failed"); > + > + if ((!WIFEXITED(status)) || (WEXITSTATUS(status) != 0)) { > + if (DEBUG) > + tst_resm(TINFO, "child exited with status %d", > + WEXITSTATUS(status)); > + > + tst_resm(TFAIL, "child failed"); > } > } > + > cleanup(); > tst_exit(); > - > } > > -/* > - * do_child() > - */ > -void do_child() > +static void do_child(void) > { > int exno = 0; > > @@ -171,15 +177,15 @@ void do_child() > tst_resm(TFAIL, "setsid failed, errno :%d", errno); > exno = 1; > } > + > + kill(getppid(), SIGUSR1); > + > sleep(2); > + > exit(exno); > } > > -/* > - * setup() > - * performs all ONE TIME setup for this test > - */ > -void setup(void) > +static void setup(void) > { > > tst_sig(FORK, DEF_HANDLER, cleanup); > @@ -189,17 +195,7 @@ void setup(void) > TEST_PAUSE; > } > > -/* > - * cleanup() > - * performs all the ONE TIME cleanup for this test at completion > - * or premature exit > - */ > -void cleanup(void) > +static void cleanup(void) > { > - /* > - * print timing status if that option was specified > - * print errno log if that option was specified > - */ > TEST_CLEANUP; > - > } > -- > 1.7.1 > > > ------------------------------------------------------------------------------ > Android is increasing in popularity, but the open development platform that > developers love is also attractive to malware creators. Download this white > paper to learn more about secure code signing practices that can help keep > Android apps secure. > http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk > _______________________________________________ > Ltp-list mailing list > Ltp...@li... > https://lists.sourceforge.net/lists/listinfo/ltp-list > |
From: Stanislav K. <sta...@or...> - 2013-10-29 11:54:21
|
On 10/29/2013 02:30 PM, Jan Stancek wrote: > Hi, Hi. > > it looks race-y after patch too. For example, let assume child runs first: > > diff --git a/testcases/kernel/syscalls/setpgid/setpgid03.c b/testcases/kernel/syscalls/setpgid/setpgid03.c > index 17fd16f..91f9940 100644 > --- a/testcases/kernel/syscalls/setpgid/setpgid03.c > +++ b/testcases/kernel/syscalls/setpgid/setpgid03.c > @@ -82,6 +82,7 @@ int main(int ac, char **av) > #endif > } > > + sleep(1); > if (sigwaitinfo(&sigusr_set, NULL) < 0) > tst_brkm(TFAIL | TERRNO, cleanup, > "sigwaitinfo() failed"); > > # ./setpgid03 > setpgid03 1 TBROK : unexpected signal 10 received (pid = 6763). > setpgid03 2 TBROK : Remaining cases broken Yes... I can't realize how I wanted to prevent this race if I put sigwaitinfo() after fork().... :( Sorry. > > It might be easier to use "checkpoint" system, see include/tst_checkpoint.h, > and lib/tests/tst_checkpoint_* for examples. Thank you. I will take a look. > > Regards, > Jan > > ----- Original Message ----- >> From: "Stanislav Kholmanskikh" <sta...@or...> >> To: ltp...@li... >> Cc: "vasily isaenko" <vas...@or...> >> Sent: Tuesday, 29 October, 2013 8:55:32 AM >> Subject: [LTP] [PATCH] setpgid03: use SIGUSR1 instead of sleep >> >> I came accross the following rare issue: >> >> setpgid03 1 TPASS : setpgid SUCCESS to set errno to EPERM >> setpgid03 2 TFAIL : setpgid FAILED, expect EACCES got 1 >> >> I suppose It may happen if the test host is under stress. >> To prevent this It would be better to use SIGUSR1 for parent-child >> synchronisations. >> >> Also fixed error passing from children and performed a minor cleanup. >> >> Signed-off-by: Stanislav Kholmanskikh <sta...@or...> >> --- >> testcases/kernel/syscalls/setpgid/setpgid03.c | 200 >> ++++++++++++------------- >> 1 files changed, 98 insertions(+), 102 deletions(-) >> >> diff --git a/testcases/kernel/syscalls/setpgid/setpgid03.c >> b/testcases/kernel/syscalls/setpgid/setpgid03.c >> index 4429fe4..17fd16f 100644 >> --- a/testcases/kernel/syscalls/setpgid/setpgid03.c >> +++ b/testcases/kernel/syscalls/setpgid/setpgid03.c >> @@ -1,34 +1,23 @@ >> /* >> + * Copyright (c) International Business Machines Corp., 2001 >> * >> - * Copyright (c) International Business Machines Corp., 2001 >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. >> * >> - * This program is free software; you can redistribute it and/or modify >> - * it under the terms of the GNU General Public License as published by >> - * the Free Software Foundation; either version 2 of the License, or >> - * (at your option) any later version. >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See >> + * the GNU General Public License for more details. >> * >> - * This program is distributed in the hope that it will be useful, >> - * but WITHOUT ANY WARRANTY; without even the implied warranty of >> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See >> - * the GNU General Public License for more details. >> - * >> - * You should have received a copy of the GNU General Public License >> - * along with this program; if not, write to the Free Software >> - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA >> 02110-1301 USA >> + * You should have received a copy of the GNU General Public License >> + * along with this program; if not, write to the Free Software >> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 >> USA >> */ >> >> /* >> - * NAME >> - * setpgid03.c >> - * >> - * DESCRIPTION >> - * Test to check the error and trivial conditions in setpgid system call >> - * >> - * USAGE >> - * setuid03 >> - * >> - * RESTRICTIONS >> - * This test is not completely written in the LTP format - PLEASE FIX! >> + * Test to check the error and trivial conditions in setpgid system call >> */ >> >> #define DEBUG 0 >> @@ -47,16 +36,16 @@ >> char *TCID = "setpgid03"; >> int TST_TOTAL = 1; >> >> -void do_child(void); >> -void setup(void); >> -void cleanup(void); >> +static void do_child(void); >> +static void setup(void); >> +static void cleanup(void); >> >> int main(int ac, char **av) >> { >> int pid; >> - int rval, fail = 0; >> - int ret, status; >> - int exno = 0; >> + int rval; >> + int status; >> + sigset_t sigusr_set; >> >> int lc; >> char *msg; >> @@ -68,14 +57,13 @@ int main(int ac, char **av) >> maybe_run_child(&do_child, ""); >> #endif >> >> - /* >> - * perform global setup for the test >> - */ >> setup(); >> >> + sigemptyset(&sigusr_set); >> + sigaddset(&sigusr_set, SIGUSR1); >> + >> for (lc = 0; TEST_LOOPING(lc); lc++) { >> >> - /* reset tst_count in case we are looping */ >> tst_count = 0; >> >> //test1: >> @@ -84,7 +72,7 @@ int main(int ac, char **av) >> tst_brkm(TBROK, cleanup, "fork() failed"); >> } >> >> - if (pid == 0) { /* child */ >> + if (pid == 0) { >> #ifdef UCLINUX >> if (self_exec(av[0], "") < 0) { >> tst_brkm(TBROK, cleanup, "self_exec failed"); >> @@ -92,35 +80,33 @@ int main(int ac, char **av) >> #else >> do_child(); >> #endif >> - } else { /* parent */ >> - sleep(1); >> - rval = setpgid(pid, getppid()); >> - if (errno == EPERM) { >> - tst_resm(TPASS, "setpgid SUCCESS to set " >> - "errno to EPERM"); >> - } else { >> - tst_resm(TFAIL, "setpgid FAILED, " >> - "expect %d, return %d", EPERM, errno); >> - fail = 1; >> - } >> - sleep(1); >> - if ((ret = wait(&status)) > 0) { >> - if (DEBUG) >> - tst_resm(TINFO, "Test {%d} exited " >> - "status 0x%0x", ret, status); >> - >> - if (status != 0) { >> - fail = 1; >> - } >> - } >> } >> - if (DEBUG) { >> - if (fail || exno) { >> - tst_resm(TINFO, "Test test 1: FAILED"); >> - } else { >> - tst_resm(TINFO, "Test test 1: PASSED"); >> - } >> + >> + if (sigwaitinfo(&sigusr_set, NULL) < 0) >> + tst_brkm(TFAIL | TERRNO, cleanup, >> + "sigwaitinfo() failed"); >> + >> + rval = setpgid(pid, getppid()); >> + if (errno == EPERM) { >> + tst_resm(TPASS, >> + "setpgid SUCCESS to set errno to EPERM"); >> + } else { >> + tst_resm(TFAIL, >> + "setpgid FAILED, expect %d, return %d", >> + EPERM, errno); >> + } >> + >> + if (wait(&status) < 0) >> + tst_resm(TFAIL | TERRNO, "wait() failed"); >> + >> + if (!(WIFEXITED(status)) || (WEXITSTATUS(status) != 0)) { >> + if (DEBUG) >> + tst_resm(TINFO, "child exited with status %d", >> + WEXITSTATUS(status)); >> + >> + tst_resm(TFAIL, "child failed"); >> } >> + >> //test2: >> /* >> * Value of pid matches the pid of the child process and >> @@ -131,39 +117,59 @@ int main(int ac, char **av) >> >> } >> if (pid == 0) { >> - if (execlp("sleep", "sleep", "3", NULL) < 0) { >> - perror("exec failed"); >> + char buf[10]; >> + int ret; >> + >> + ret = snprintf(buf, 10, "%d", getppid()); >> + if ((ret >= 10) || (ret <= 0)) { >> + printf("snprintf() failed\n"); >> + >> + exit(126); >> } >> + >> + const char *args[4 + 1]; >> + args[0] = "kill"; >> + args[1] = "-s"; >> + args[2] = "SIGUSR1"; >> + args[3] = buf; >> + args[4] = NULL; >> + >> + if (execvp(args[0], (char *const *)args) < 0) >> + perror("exec failed\n"); >> + >> exit(127); >> + } >> + >> + if (sigwaitinfo(&sigusr_set, NULL) < 0) >> + tst_brkm(TFAIL | TERRNO, cleanup, >> + "sigwaitinfo() failed"); >> + >> + rval = setpgid(pid, getppid()); >> + if (errno == EACCES) { >> + tst_resm(TPASS, >> + "setpgid SUCCEEDED to set errno to EACCES"); >> } else { >> - sleep(1); >> - rval = setpgid(pid, getppid()); >> - if (errno == EACCES) { >> - tst_resm(TPASS, "setpgid SUCCEEDED to set " >> - "errno to EACCES"); >> - } else { >> - tst_resm(TFAIL, "setpgid FAILED, expect EACCES " >> - "got %d", errno); >> - } >> - if ((ret = wait(&status)) > 0) { >> - if (DEBUG) >> - tst_resm(TINFO, "Test {%d} exited " >> - "status 0x%0x", ret, status); >> - if (status != 0) { >> - fail = 1; >> - } >> - } >> + tst_resm(TFAIL, >> + "setpgid FAILED, expect EACCES got %d", errno); >> + } >> + >> + if (wait(&status) < 0) >> + tst_resm(TFAIL | TERRNO, "wait() failed"); >> + >> + if ((!WIFEXITED(status)) || (WEXITSTATUS(status) != 0)) { >> + if (DEBUG) >> + tst_resm(TINFO, "child exited with status %d", >> + WEXITSTATUS(status)); >> + >> + tst_resm(TFAIL, "child failed"); >> } >> } >> + >> cleanup(); >> tst_exit(); >> - >> } >> >> -/* >> - * do_child() >> - */ >> -void do_child() >> +static void do_child(void) >> { >> int exno = 0; >> >> @@ -171,15 +177,15 @@ void do_child() >> tst_resm(TFAIL, "setsid failed, errno :%d", errno); >> exno = 1; >> } >> + >> + kill(getppid(), SIGUSR1); >> + >> sleep(2); >> + >> exit(exno); >> } >> >> -/* >> - * setup() >> - * performs all ONE TIME setup for this test >> - */ >> -void setup(void) >> +static void setup(void) >> { >> >> tst_sig(FORK, DEF_HANDLER, cleanup); >> @@ -189,17 +195,7 @@ void setup(void) >> TEST_PAUSE; >> } >> >> -/* >> - * cleanup() >> - * performs all the ONE TIME cleanup for this test at completion >> - * or premature exit >> - */ >> -void cleanup(void) >> +static void cleanup(void) >> { >> - /* >> - * print timing status if that option was specified >> - * print errno log if that option was specified >> - */ >> TEST_CLEANUP; >> - >> } >> -- >> 1.7.1 >> >> >> ------------------------------------------------------------------------------ >> Android is increasing in popularity, but the open development platform that >> developers love is also attractive to malware creators. Download this white >> paper to learn more about secure code signing practices that can help keep >> Android apps secure. >> http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk >> _______________________________________________ >> Ltp-list mailing list >> Ltp...@li... >> https://lists.sourceforge.net/lists/listinfo/ltp-list >> |
From: Stanislav K. <sta...@or...> - 2013-10-30 09:07:55
|
Hi! Changes since V1: * Divided cleanup and logic parts of my previous patch into two separate patches * It seems that "checkpoint" system is not appropriate for this case, because in test2 we need to use exec(). Therefore I use signals. Thanks. |
From: Stanislav K. <sta...@or...> - 2013-10-30 09:07:58
|
I came accross the following rare issue: setpgid03 1 TPASS : setpgid SUCCESS to set errno to EPERM setpgid03 2 TFAIL : setpgid FAILED, expect EACCES got 1 I suppose It may happen if the test host is under stress. To prevent this It would be better to use SIGUSR1 for parent-child synchronisations. Signed-off-by: Stanislav Kholmanskikh <sta...@or...> --- testcases/kernel/syscalls/setpgid/setpgid03.c | 123 +++++++++++++++++++++++-- 1 files changed, 115 insertions(+), 8 deletions(-) diff --git a/testcases/kernel/syscalls/setpgid/setpgid03.c b/testcases/kernel/syscalls/setpgid/setpgid03.c index 2c47bc4..34975b7 100644 --- a/testcases/kernel/syscalls/setpgid/setpgid03.c +++ b/testcases/kernel/syscalls/setpgid/setpgid03.c @@ -1,5 +1,6 @@ /* * Copyright (c) International Business Machines Corp., 2001 + * Copyright (c) 2013 Oracle and/or its affiliates. All Rights Reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -34,15 +35,20 @@ char *TCID = "setpgid03"; int TST_TOTAL = 1; +static pid_t pid = -1; + static void do_child(void); +static void sigusr_handler(int signum); +static void term_handler(int signum); static void setup(void); static void cleanup(void); int main(int ac, char **av) { - int pid; int rval; int status; + sigset_t sig_set; + struct sigaction sa; int lc; char *msg; @@ -56,12 +62,42 @@ int main(int ac, char **av) setup(); + /* We need to block these signals before fork() + * because our children may terminate either + * normally (parent receives both SIGUSR1 and SIGCHLD) or + * abnormally (parent receives SIGCHLD only) + */ + sigemptyset(&sig_set); + sigaddset(&sig_set, SIGUSR1); + sigaddset(&sig_set, SIGCHLD); + + /* Since we use SIGUSR1 as a control signal It should not + * be an "unexpected" one + */ + memset(&sa, 0, sizeof(sa)); + sa.sa_handler = sigusr_handler; + if (sigaction(SIGUSR1, &sa, NULL) < 0) + tst_brkm(TFAIL | TERRNO, cleanup, "sigaction(SIGUSR1) failed"); + + /* But for SIGINT, SIGTERM we should kill our children */ + sa.sa_handler = term_handler; + if (sigaction(SIGTERM, &sa, NULL) < 0) + tst_brkm(TFAIL | TERRNO, cleanup, "sigaction(SIGTERM) failed"); + if (sigaction(SIGINT, &sa, NULL) < 0) + tst_brkm(TFAIL | TERRNO, cleanup, "sigaction(SIGINT) failed"); + for (lc = 0; TEST_LOOPING(lc); lc++) { tst_count = 0; //test1: /* sid of the calling process is not same */ + + if (sigprocmask(SIG_BLOCK, &sig_set, NULL) < 0) + tst_brkm(TFAIL | TERRNO, cleanup, + "sigprocmask(SIG_BLOCK) in test 1 failed"); + + if ((pid = FORK_OR_VFORK()) == -1) { tst_brkm(TBROK, cleanup, "fork() failed"); } @@ -76,7 +112,14 @@ int main(int ac, char **av) #endif } - sleep(1); + if (sigwaitinfo(&sig_set, NULL) < 0) + tst_brkm(TFAIL | TERRNO, cleanup, + "sigwaitinfo() failed"); + + if (sigprocmask(SIG_UNBLOCK, &sig_set, NULL) < 0) + tst_brkm(TFAIL | TERRNO, cleanup, + "sigprocmask(SIG_UNBLOCK) in test 2 failed"); + rval = setpgid(pid, getppid()); if (errno == EPERM) { tst_resm(TPASS, @@ -86,11 +129,14 @@ int main(int ac, char **av) "setpgid FAILED, expect %d, return %d", EPERM, errno); } - sleep(1); + + kill(pid, SIGUSR1); if (wait(&status) < 0) tst_resm(TFAIL | TERRNO, "wait() for child 1 failed"); + pid = -1; + if (!(WIFEXITED(status)) || (WEXITSTATUS(status) != 0)) tst_resm(TFAIL, "child 1 failed with status %d", WEXITSTATUS(status)); @@ -100,18 +146,47 @@ int main(int ac, char **av) * Value of pid matches the pid of the child process and * the child process has exec successfully. Error */ + + if (sigprocmask(SIG_BLOCK, &sig_set, NULL) < 0) + tst_brkm(TFAIL | TERRNO, cleanup, + "sigprocmask(SIG_BLOCK) in test 2 failed"); + if ((pid = FORK_OR_VFORK()) == -1) { tst_resm(TFAIL, "Fork failed"); } if (pid == 0) { - if (execlp("sleep", "sleep", "3", NULL) < 0) { - perror("exec failed"); + char buf[10]; + int ret; + + ret = snprintf(buf, 10, "%d", getppid()); + if ((ret >= 10) || (ret <= 0)) { + printf("snprintf() failed\n"); + + exit(126); } + + const char *args[4 + 1]; + args[0] = "kill"; + args[1] = "-s"; + args[2] = "SIGUSR1"; + args[3] = buf; + args[4] = NULL; + + if (execvp(args[0], (char *const *)args) < 0) + perror("execvp() failed\n"); + exit(127); } - sleep(1); + if (sigwaitinfo(&sig_set, NULL) < 0) + tst_brkm(TFAIL | TERRNO, cleanup, + "sigwaitinfo() failed"); + + if (sigprocmask(SIG_UNBLOCK, &sig_set, NULL) < 0) + tst_brkm(TFAIL | TERRNO, cleanup, + "sigprocmask(SIG_UNBLOCK) in test 2 failed"); + rval = setpgid(pid, getppid()); if (errno == EACCES) { tst_resm(TPASS, @@ -124,6 +199,8 @@ int main(int ac, char **av) if (wait(&status) < 0) tst_resm(TFAIL | TERRNO, "wait() for child 2 failed"); + pid = -1; + if (!(WIFEXITED(status)) || (WEXITSTATUS(status) != 0)) tst_resm(TFAIL, "child 2 failed with status %d", WEXITSTATUS(status)); @@ -136,18 +213,48 @@ int main(int ac, char **av) static void do_child(void) { int exno = 0; + sigset_t set; if (setsid() < 0) { printf("setsid() failed, errno: %d\n", errno); exno = 1; } - sleep(2); + + kill(getppid(), SIGUSR1); + + sigemptyset(&set); + sigaddset(&set, SIGUSR1); + if (sigprocmask(SIG_UNBLOCK, &set, NULL) < 0) { + printf("sigprocmask() in child failed: %d\n", errno); + + exit(1); + } + + pause(); + exit(exno); } -static void setup(void) +static void sigusr_handler(int signum) { +} + +static void term_handler(int signum) +{ + if (pid == 0) { + printf("Child received signal %d\n", signum); + exit(1); + } else { + if (pid > 0) + kill(pid, signum); + + tst_brkm(TFAIL, cleanup, "Signal %d received. Exiting", signum); + } +} + +static void setup(void) +{ tst_sig(FORK, DEF_HANDLER, cleanup); umask(0); -- 1.7.1 |
From: Stanislav K. <sta...@or...> - 2013-12-12 09:22:10
|
If one of the children exits with non zero value we can't see it until DEBUG is set. Now if the exit value is not 0, we print this value with tst_resm(TFAIL). Also performed minor cleanup. Signed-off-by: Stanislav Kholmanskikh <sta...@or...> --- testcases/kernel/syscalls/setpgid/setpgid03.c | 160 +++++++++---------------- 1 files changed, 58 insertions(+), 102 deletions(-) diff --git a/testcases/kernel/syscalls/setpgid/setpgid03.c b/testcases/kernel/syscalls/setpgid/setpgid03.c index 4429fe4..2c47bc4 100644 --- a/testcases/kernel/syscalls/setpgid/setpgid03.c +++ b/testcases/kernel/syscalls/setpgid/setpgid03.c @@ -1,38 +1,25 @@ /* + * Copyright (c) International Business Machines Corp., 2001 * - * Copyright (c) International Business Machines Corp., 2001 + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See + * the GNU General Public License for more details. * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See - * the GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA */ /* - * NAME - * setpgid03.c - * - * DESCRIPTION - * Test to check the error and trivial conditions in setpgid system call - * - * USAGE - * setuid03 - * - * RESTRICTIONS - * This test is not completely written in the LTP format - PLEASE FIX! + * Test to check the error and trivial conditions in setpgid system call */ -#define DEBUG 0 - #include <wait.h> #include <limits.h> #include <signal.h> @@ -47,16 +34,15 @@ char *TCID = "setpgid03"; int TST_TOTAL = 1; -void do_child(void); -void setup(void); -void cleanup(void); +static void do_child(void); +static void setup(void); +static void cleanup(void); int main(int ac, char **av) { int pid; - int rval, fail = 0; - int ret, status; - int exno = 0; + int rval; + int status; int lc; char *msg; @@ -68,14 +54,10 @@ int main(int ac, char **av) maybe_run_child(&do_child, ""); #endif - /* - * perform global setup for the test - */ setup(); for (lc = 0; TEST_LOOPING(lc); lc++) { - /* reset tst_count in case we are looping */ tst_count = 0; //test1: @@ -92,35 +74,27 @@ int main(int ac, char **av) #else do_child(); #endif - } else { /* parent */ - sleep(1); - rval = setpgid(pid, getppid()); - if (errno == EPERM) { - tst_resm(TPASS, "setpgid SUCCESS to set " - "errno to EPERM"); - } else { - tst_resm(TFAIL, "setpgid FAILED, " - "expect %d, return %d", EPERM, errno); - fail = 1; - } - sleep(1); - if ((ret = wait(&status)) > 0) { - if (DEBUG) - tst_resm(TINFO, "Test {%d} exited " - "status 0x%0x", ret, status); - - if (status != 0) { - fail = 1; - } - } } - if (DEBUG) { - if (fail || exno) { - tst_resm(TINFO, "Test test 1: FAILED"); - } else { - tst_resm(TINFO, "Test test 1: PASSED"); - } + + sleep(1); + rval = setpgid(pid, getppid()); + if (errno == EPERM) { + tst_resm(TPASS, + "setpgid SUCCESS to set errno to EPERM"); + } else { + tst_resm(TFAIL, + "setpgid FAILED, expect %d, return %d", + EPERM, errno); } + sleep(1); + + if (wait(&status) < 0) + tst_resm(TFAIL | TERRNO, "wait() for child 1 failed"); + + if (!(WIFEXITED(status)) || (WEXITSTATUS(status) != 0)) + tst_resm(TFAIL, "child 1 failed with status %d", + WEXITSTATUS(status)); + //test2: /* * Value of pid matches the pid of the child process and @@ -135,51 +109,43 @@ int main(int ac, char **av) perror("exec failed"); } exit(127); + } + + sleep(1); + rval = setpgid(pid, getppid()); + if (errno == EACCES) { + tst_resm(TPASS, + "setpgid SUCCEEDED to set errno to EACCES"); } else { - sleep(1); - rval = setpgid(pid, getppid()); - if (errno == EACCES) { - tst_resm(TPASS, "setpgid SUCCEEDED to set " - "errno to EACCES"); - } else { - tst_resm(TFAIL, "setpgid FAILED, expect EACCES " - "got %d", errno); - } - if ((ret = wait(&status)) > 0) { - if (DEBUG) - tst_resm(TINFO, "Test {%d} exited " - "status 0x%0x", ret, status); - if (status != 0) { - fail = 1; - } - } + tst_resm(TFAIL, + "setpgid FAILED, expect EACCES got %d", errno); } + + if (wait(&status) < 0) + tst_resm(TFAIL | TERRNO, "wait() for child 2 failed"); + + if (!(WIFEXITED(status)) || (WEXITSTATUS(status) != 0)) + tst_resm(TFAIL, "child 2 failed with status %d", + WEXITSTATUS(status)); } + cleanup(); tst_exit(); - } -/* - * do_child() - */ -void do_child() +static void do_child(void) { int exno = 0; if (setsid() < 0) { - tst_resm(TFAIL, "setsid failed, errno :%d", errno); + printf("setsid() failed, errno: %d\n", errno); exno = 1; } sleep(2); exit(exno); } -/* - * setup() - * performs all ONE TIME setup for this test - */ -void setup(void) +static void setup(void) { tst_sig(FORK, DEF_HANDLER, cleanup); @@ -189,17 +155,7 @@ void setup(void) TEST_PAUSE; } -/* - * cleanup() - * performs all the ONE TIME cleanup for this test at completion - * or premature exit - */ -void cleanup(void) +static void cleanup(void) { - /* - * print timing status if that option was specified - * print errno log if that option was specified - */ TEST_CLEANUP; - } -- 1.7.1 |
From: Stanislav K. <sta...@or...> - 2013-12-12 09:22:10
|
I came accross the following rare issue: setpgid03 1 TPASS : setpgid SUCCESS to set errno to EPERM setpgid03 2 TFAIL : setpgid FAILED, expect EACCES got 1 I suppose it may happen if the test host is under stress. To prevent this it would be better to use tst_checkpoint interface for parent-child synchronisations. Signed-off-by: Stanislav Kholmanskikh <sta...@or...> --- testcases/kernel/syscalls/.gitignore | 1 + testcases/kernel/syscalls/setpgid/setpgid03.c | 57 ++++++++++++++++---- .../kernel/syscalls/setpgid/setpgid03_child.c | 31 +++++++++++ 3 files changed, 78 insertions(+), 11 deletions(-) create mode 100644 testcases/kernel/syscalls/setpgid/setpgid03_child.c diff --git a/testcases/kernel/syscalls/.gitignore b/testcases/kernel/syscalls/.gitignore index bbefb25..6ce8900 100644 --- a/testcases/kernel/syscalls/.gitignore +++ b/testcases/kernel/syscalls/.gitignore @@ -777,6 +777,7 @@ /setpgid/setpgid01 /setpgid/setpgid02 /setpgid/setpgid03 +/setpgid/setpgid03_child /setpgrp/setpgrp01 /setpgrp/setpgrp02 /setpriority/setpriority01 diff --git a/testcases/kernel/syscalls/setpgid/setpgid03.c b/testcases/kernel/syscalls/setpgid/setpgid03.c index 2c47bc4..b9ecddd 100644 --- a/testcases/kernel/syscalls/setpgid/setpgid03.c +++ b/testcases/kernel/syscalls/setpgid/setpgid03.c @@ -1,5 +1,6 @@ /* * Copyright (c) International Business Machines Corp., 2001 + * Copyright (c) 2013 Oracle and/or its affiliates. All Rights Reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -31,12 +32,17 @@ #include "test.h" #include "usctest.h" +#define TEST_APP "setpgid03_child" + char *TCID = "setpgid03"; int TST_TOTAL = 1; +static struct tst_checkpoint checkpoint; + static void do_child(void); static void setup(void); static void cleanup(void); +static void alarm_handler(int signo); int main(int ac, char **av) { @@ -76,7 +82,7 @@ int main(int ac, char **av) #endif } - sleep(1); + TST_CHECKPOINT_PARENT_WAIT(cleanup, &checkpoint); rval = setpgid(pid, getppid()); if (errno == EPERM) { tst_resm(TPASS, @@ -86,7 +92,7 @@ int main(int ac, char **av) "setpgid FAILED, expect %d, return %d", EPERM, errno); } - sleep(1); + TST_CHECKPOINT_SIGNAL_CHILD(cleanup, &checkpoint); if (wait(&status) < 0) tst_resm(TFAIL | TERRNO, "wait() for child 1 failed"); @@ -105,13 +111,13 @@ int main(int ac, char **av) } if (pid == 0) { - if (execlp("sleep", "sleep", "3", NULL) < 0) { + if (execlp(TEST_APP, TEST_APP, NULL) < 0) perror("exec failed"); - } + exit(127); } - sleep(1); + TST_CHECKPOINT_PARENT_WAIT(cleanup, &checkpoint); rval = setpgid(pid, getppid()); if (errno == EACCES) { tst_resm(TPASS, @@ -120,6 +126,7 @@ int main(int ac, char **av) tst_resm(TFAIL, "setpgid FAILED, expect EACCES got %d", errno); } + TST_CHECKPOINT_SIGNAL_CHILD(cleanup, &checkpoint); if (wait(&status) < 0) tst_resm(TFAIL | TERRNO, "wait() for child 2 failed"); @@ -133,23 +140,49 @@ int main(int ac, char **av) tst_exit(); } +static void alarm_handler(int signo) +{ + printf("CHILD: checkpoint timed out\n"); + + exit(3); +} + static void do_child(void) { - int exno = 0; + unsigned int timeout = checkpoint.timeout / 1000; + struct sigaction sa; + + memset(&sa, 0, sizeof(struct sigaction)); + sa.sa_handler = alarm_handler; + + if (sigaction(SIGALRM, &sa, NULL) < 0) { + printf("CHILD: sigaction() failed, errno: %d\n", errno); + exit(1); + } if (setsid() < 0) { - printf("setsid() failed, errno: %d\n", errno); - exno = 1; + printf("CHILD: setsid() failed, errno: %d\n", errno); + exit(2); } - sleep(2); - exit(exno); + + alarm(timeout); + TST_CHECKPOINT_SIGNAL_PARENT(&checkpoint); + + alarm(timeout); + TST_CHECKPOINT_CHILD_WAIT(&checkpoint); + + exit(0); } static void setup(void) { - tst_sig(FORK, DEF_HANDLER, cleanup); + tst_tmpdir(); + + TST_CHECKPOINT_INIT(&checkpoint); + checkpoint.timeout = 10000; + umask(0); TEST_PAUSE; @@ -157,5 +190,7 @@ static void setup(void) static void cleanup(void) { + tst_rmdir(); + TEST_CLEANUP; } diff --git a/testcases/kernel/syscalls/setpgid/setpgid03_child.c b/testcases/kernel/syscalls/setpgid/setpgid03_child.c new file mode 100644 index 0000000..c4cf892 --- /dev/null +++ b/testcases/kernel/syscalls/setpgid/setpgid03_child.c @@ -0,0 +1,31 @@ +/* + * Copyright (c) 2013 Oracle and/or its affiliates. All Rights Reserved. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it would be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include "test.h" + +char *TCID = "setpgid03_child"; + +static struct tst_checkpoint checkpoint = { .timeout = 10000, .retval = 1}; + +int main(void) +{ + TST_CHECKPOINT_SIGNAL_PARENT(&checkpoint); + TST_CHECKPOINT_CHILD_WAIT(&checkpoint); + + return 0; +} -- 1.7.1 |
From: <ch...@su...> - 2014-01-20 16:09:55
|
Hi! > I came accross the following rare issue: > > setpgid03 1 TPASS : setpgid SUCCESS to set errno to EPERM > setpgid03 2 TFAIL : setpgid FAILED, expect EACCES got 1 > > I suppose it may happen if the test host is under stress. > > To prevent this it would be better to use tst_checkpoint interface > for parent-child synchronisations. Pushed. I've added a follow up patch that removes the alarm() timeout code in the child and does a few more fixes. The timeout should better be handled, if handled at all, in the checkpoint code. I would not care much about this case because when the parent is dead does not talk to the child the test will fail anyway... -- Cyril Hrubis ch...@su... |
From: Stanislav K. <sta...@or...> - 2014-01-21 13:04:50
|
On 01/20/2014 08:09 PM, ch...@su... wrote: > Hi! >> I came accross the following rare issue: >> >> setpgid03 1 TPASS : setpgid SUCCESS to set errno to EPERM >> setpgid03 2 TFAIL : setpgid FAILED, expect EACCES got 1 >> >> I suppose it may happen if the test host is under stress. >> >> To prevent this it would be better to use tst_checkpoint interface >> for parent-child synchronisations. > > Pushed. > > I've added a follow up patch that removes the alarm() timeout code in > the child and does a few more fixes. Thank you. > > The timeout should better be handled, if handled at all, in the > checkpoint code. I would not care much about this case because when the > parent is dead does not talk to the child the test will fail anyway... > I didn't care about the test failure in such case. I cared about that without alarm() the child will keep running (i.e. blocking on IO) even after the parent is dead. But I agree that it's not the best place to handle this inside the test case. |
From: <ch...@su...> - 2014-01-21 13:21:25
|
Hi! > > The timeout should better be handled, if handled at all, in the > > checkpoint code. I would not care much about this case because when the > > parent is dead does not talk to the child the test will fail anyway... > > > > I didn't care about the test failure in such case. I cared about that > without alarm() the child will keep running (i.e. blocking on IO) even > after the parent is dead. I've noticed that as well, the child will continue to run and because it called setsid() the test driver (pan) will not kill it because it left it's parent process group. > But I agree that it's not the best place to handle this inside the test > case. I was thinking of adding the alarm() code into the checkpoint child code. This should fix the problem once for all. But even then this codepath happens only when something went wrong so this has not big priority... -- Cyril Hrubis ch...@su... |
From: Stanislav K. <sta...@or...> - 2013-10-30 09:07:56
|
If one of the children exits with non zero value we can't see it until DEBUG is set. Now if the exit value is not 0, we print this value with tst_resm(TFAIL). Also performed minor cleanup. Signed-off-by: Stanislav Kholmanskikh <sta...@or...> --- testcases/kernel/syscalls/setpgid/setpgid03.c | 160 +++++++++---------------- 1 files changed, 58 insertions(+), 102 deletions(-) diff --git a/testcases/kernel/syscalls/setpgid/setpgid03.c b/testcases/kernel/syscalls/setpgid/setpgid03.c index 4429fe4..2c47bc4 100644 --- a/testcases/kernel/syscalls/setpgid/setpgid03.c +++ b/testcases/kernel/syscalls/setpgid/setpgid03.c @@ -1,38 +1,25 @@ /* + * Copyright (c) International Business Machines Corp., 2001 * - * Copyright (c) International Business Machines Corp., 2001 + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See + * the GNU General Public License for more details. * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See - * the GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA */ /* - * NAME - * setpgid03.c - * - * DESCRIPTION - * Test to check the error and trivial conditions in setpgid system call - * - * USAGE - * setuid03 - * - * RESTRICTIONS - * This test is not completely written in the LTP format - PLEASE FIX! + * Test to check the error and trivial conditions in setpgid system call */ -#define DEBUG 0 - #include <wait.h> #include <limits.h> #include <signal.h> @@ -47,16 +34,15 @@ char *TCID = "setpgid03"; int TST_TOTAL = 1; -void do_child(void); -void setup(void); -void cleanup(void); +static void do_child(void); +static void setup(void); +static void cleanup(void); int main(int ac, char **av) { int pid; - int rval, fail = 0; - int ret, status; - int exno = 0; + int rval; + int status; int lc; char *msg; @@ -68,14 +54,10 @@ int main(int ac, char **av) maybe_run_child(&do_child, ""); #endif - /* - * perform global setup for the test - */ setup(); for (lc = 0; TEST_LOOPING(lc); lc++) { - /* reset tst_count in case we are looping */ tst_count = 0; //test1: @@ -92,35 +74,27 @@ int main(int ac, char **av) #else do_child(); #endif - } else { /* parent */ - sleep(1); - rval = setpgid(pid, getppid()); - if (errno == EPERM) { - tst_resm(TPASS, "setpgid SUCCESS to set " - "errno to EPERM"); - } else { - tst_resm(TFAIL, "setpgid FAILED, " - "expect %d, return %d", EPERM, errno); - fail = 1; - } - sleep(1); - if ((ret = wait(&status)) > 0) { - if (DEBUG) - tst_resm(TINFO, "Test {%d} exited " - "status 0x%0x", ret, status); - - if (status != 0) { - fail = 1; - } - } } - if (DEBUG) { - if (fail || exno) { - tst_resm(TINFO, "Test test 1: FAILED"); - } else { - tst_resm(TINFO, "Test test 1: PASSED"); - } + + sleep(1); + rval = setpgid(pid, getppid()); + if (errno == EPERM) { + tst_resm(TPASS, + "setpgid SUCCESS to set errno to EPERM"); + } else { + tst_resm(TFAIL, + "setpgid FAILED, expect %d, return %d", + EPERM, errno); } + sleep(1); + + if (wait(&status) < 0) + tst_resm(TFAIL | TERRNO, "wait() for child 1 failed"); + + if (!(WIFEXITED(status)) || (WEXITSTATUS(status) != 0)) + tst_resm(TFAIL, "child 1 failed with status %d", + WEXITSTATUS(status)); + //test2: /* * Value of pid matches the pid of the child process and @@ -135,51 +109,43 @@ int main(int ac, char **av) perror("exec failed"); } exit(127); + } + + sleep(1); + rval = setpgid(pid, getppid()); + if (errno == EACCES) { + tst_resm(TPASS, + "setpgid SUCCEEDED to set errno to EACCES"); } else { - sleep(1); - rval = setpgid(pid, getppid()); - if (errno == EACCES) { - tst_resm(TPASS, "setpgid SUCCEEDED to set " - "errno to EACCES"); - } else { - tst_resm(TFAIL, "setpgid FAILED, expect EACCES " - "got %d", errno); - } - if ((ret = wait(&status)) > 0) { - if (DEBUG) - tst_resm(TINFO, "Test {%d} exited " - "status 0x%0x", ret, status); - if (status != 0) { - fail = 1; - } - } + tst_resm(TFAIL, + "setpgid FAILED, expect EACCES got %d", errno); } + + if (wait(&status) < 0) + tst_resm(TFAIL | TERRNO, "wait() for child 2 failed"); + + if (!(WIFEXITED(status)) || (WEXITSTATUS(status) != 0)) + tst_resm(TFAIL, "child 2 failed with status %d", + WEXITSTATUS(status)); } + cleanup(); tst_exit(); - } -/* - * do_child() - */ -void do_child() +static void do_child(void) { int exno = 0; if (setsid() < 0) { - tst_resm(TFAIL, "setsid failed, errno :%d", errno); + printf("setsid() failed, errno: %d\n", errno); exno = 1; } sleep(2); exit(exno); } -/* - * setup() - * performs all ONE TIME setup for this test - */ -void setup(void) +static void setup(void) { tst_sig(FORK, DEF_HANDLER, cleanup); @@ -189,17 +155,7 @@ void setup(void) TEST_PAUSE; } -/* - * cleanup() - * performs all the ONE TIME cleanup for this test at completion - * or premature exit - */ -void cleanup(void) +static void cleanup(void) { - /* - * print timing status if that option was specified - * print errno log if that option was specified - */ TEST_CLEANUP; - } -- 1.7.1 |
From: Jan S. <jst...@re...> - 2013-10-30 13:45:42
|
Hi, I think I still see a possible race, try running it with following change: diff --git a/testcases/kernel/syscalls/setpgid/setpgid03.c b/testcases/kernel/syscalls/setpgid/setpgid03.c index 34975b7..1d478cb 100644 --- a/testcases/kernel/syscalls/setpgid/setpgid03.c +++ b/testcases/kernel/syscalls/setpgid/setpgid03.c @@ -221,6 +221,7 @@ static void do_child(void) } kill(getppid(), SIGUSR1); + sleep(1); sigemptyset(&set); sigaddset(&set, SIGUSR1); I also noticed that test2 can now call setpgid() on process which can be zombie at that time. It worked fine on my system (RHEL6.4), but it made me wonder if we can rely on this behavior. If we can, then also test1's do_child() can be simplified. Regards, Jan ----- Original Message ----- > From: "Stanislav Kholmanskikh" <sta...@or...> > To: ltp...@li... > Cc: "vasily isaenko" <vas...@or...> > Sent: Wednesday, 30 October, 2013 10:07:38 AM > Subject: [LTP] [PATCH V2 2/2] setpgid03: use SIGUSR1 instead of sleep > > I came accross the following rare issue: > > setpgid03 1 TPASS : setpgid SUCCESS to set errno to EPERM > setpgid03 2 TFAIL : setpgid FAILED, expect EACCES got 1 > > I suppose It may happen if the test host is under stress. > > To prevent this It would be better to use SIGUSR1 for parent-child > synchronisations. > > Signed-off-by: Stanislav Kholmanskikh <sta...@or...> > --- > testcases/kernel/syscalls/setpgid/setpgid03.c | 123 > +++++++++++++++++++++++-- > 1 files changed, 115 insertions(+), 8 deletions(-) > > diff --git a/testcases/kernel/syscalls/setpgid/setpgid03.c > b/testcases/kernel/syscalls/setpgid/setpgid03.c > index 2c47bc4..34975b7 100644 > --- a/testcases/kernel/syscalls/setpgid/setpgid03.c > +++ b/testcases/kernel/syscalls/setpgid/setpgid03.c > @@ -1,5 +1,6 @@ > /* > * Copyright (c) International Business Machines Corp., 2001 > + * Copyright (c) 2013 Oracle and/or its affiliates. All Rights Reserved. > * > * This program is free software; you can redistribute it and/or modify > * it under the terms of the GNU General Public License as published by > @@ -34,15 +35,20 @@ > char *TCID = "setpgid03"; > int TST_TOTAL = 1; > > +static pid_t pid = -1; > + > static void do_child(void); > +static void sigusr_handler(int signum); > +static void term_handler(int signum); > static void setup(void); > static void cleanup(void); > > int main(int ac, char **av) > { > - int pid; > int rval; > int status; > + sigset_t sig_set; > + struct sigaction sa; > > int lc; > char *msg; > @@ -56,12 +62,42 @@ int main(int ac, char **av) > > setup(); > > + /* We need to block these signals before fork() > + * because our children may terminate either > + * normally (parent receives both SIGUSR1 and SIGCHLD) or > + * abnormally (parent receives SIGCHLD only) > + */ > + sigemptyset(&sig_set); > + sigaddset(&sig_set, SIGUSR1); > + sigaddset(&sig_set, SIGCHLD); > + > + /* Since we use SIGUSR1 as a control signal It should not > + * be an "unexpected" one > + */ > + memset(&sa, 0, sizeof(sa)); > + sa.sa_handler = sigusr_handler; > + if (sigaction(SIGUSR1, &sa, NULL) < 0) > + tst_brkm(TFAIL | TERRNO, cleanup, "sigaction(SIGUSR1) failed"); > + > + /* But for SIGINT, SIGTERM we should kill our children */ > + sa.sa_handler = term_handler; > + if (sigaction(SIGTERM, &sa, NULL) < 0) > + tst_brkm(TFAIL | TERRNO, cleanup, "sigaction(SIGTERM) failed"); > + if (sigaction(SIGINT, &sa, NULL) < 0) > + tst_brkm(TFAIL | TERRNO, cleanup, "sigaction(SIGINT) failed"); > + > for (lc = 0; TEST_LOOPING(lc); lc++) { > > tst_count = 0; > > //test1: > /* sid of the calling process is not same */ > + > + if (sigprocmask(SIG_BLOCK, &sig_set, NULL) < 0) > + tst_brkm(TFAIL | TERRNO, cleanup, > + "sigprocmask(SIG_BLOCK) in test 1 failed"); > + > + > if ((pid = FORK_OR_VFORK()) == -1) { > tst_brkm(TBROK, cleanup, "fork() failed"); > } > @@ -76,7 +112,14 @@ int main(int ac, char **av) > #endif > } > > - sleep(1); > + if (sigwaitinfo(&sig_set, NULL) < 0) > + tst_brkm(TFAIL | TERRNO, cleanup, > + "sigwaitinfo() failed"); > + > + if (sigprocmask(SIG_UNBLOCK, &sig_set, NULL) < 0) > + tst_brkm(TFAIL | TERRNO, cleanup, > + "sigprocmask(SIG_UNBLOCK) in test 2 failed"); > + > rval = setpgid(pid, getppid()); > if (errno == EPERM) { > tst_resm(TPASS, > @@ -86,11 +129,14 @@ int main(int ac, char **av) > "setpgid FAILED, expect %d, return %d", > EPERM, errno); > } > - sleep(1); > + > + kill(pid, SIGUSR1); > > if (wait(&status) < 0) > tst_resm(TFAIL | TERRNO, "wait() for child 1 failed"); > > + pid = -1; > + > if (!(WIFEXITED(status)) || (WEXITSTATUS(status) != 0)) > tst_resm(TFAIL, "child 1 failed with status %d", > WEXITSTATUS(status)); > @@ -100,18 +146,47 @@ int main(int ac, char **av) > * Value of pid matches the pid of the child process and > * the child process has exec successfully. Error > */ > + > + if (sigprocmask(SIG_BLOCK, &sig_set, NULL) < 0) > + tst_brkm(TFAIL | TERRNO, cleanup, > + "sigprocmask(SIG_BLOCK) in test 2 failed"); > + > if ((pid = FORK_OR_VFORK()) == -1) { > tst_resm(TFAIL, "Fork failed"); > > } > if (pid == 0) { > - if (execlp("sleep", "sleep", "3", NULL) < 0) { > - perror("exec failed"); > + char buf[10]; > + int ret; > + > + ret = snprintf(buf, 10, "%d", getppid()); > + if ((ret >= 10) || (ret <= 0)) { > + printf("snprintf() failed\n"); > + > + exit(126); > } > + > + const char *args[4 + 1]; > + args[0] = "kill"; > + args[1] = "-s"; > + args[2] = "SIGUSR1"; > + args[3] = buf; > + args[4] = NULL; > + > + if (execvp(args[0], (char *const *)args) < 0) > + perror("execvp() failed\n"); > + > exit(127); > } > > - sleep(1); > + if (sigwaitinfo(&sig_set, NULL) < 0) > + tst_brkm(TFAIL | TERRNO, cleanup, > + "sigwaitinfo() failed"); > + > + if (sigprocmask(SIG_UNBLOCK, &sig_set, NULL) < 0) > + tst_brkm(TFAIL | TERRNO, cleanup, > + "sigprocmask(SIG_UNBLOCK) in test 2 failed"); > + > rval = setpgid(pid, getppid()); > if (errno == EACCES) { > tst_resm(TPASS, > @@ -124,6 +199,8 @@ int main(int ac, char **av) > if (wait(&status) < 0) > tst_resm(TFAIL | TERRNO, "wait() for child 2 failed"); > > + pid = -1; > + > if (!(WIFEXITED(status)) || (WEXITSTATUS(status) != 0)) > tst_resm(TFAIL, "child 2 failed with status %d", > WEXITSTATUS(status)); > @@ -136,18 +213,48 @@ int main(int ac, char **av) > static void do_child(void) > { > int exno = 0; > + sigset_t set; > > if (setsid() < 0) { > printf("setsid() failed, errno: %d\n", errno); > exno = 1; > } > - sleep(2); > + > + kill(getppid(), SIGUSR1); > + > + sigemptyset(&set); > + sigaddset(&set, SIGUSR1); > + if (sigprocmask(SIG_UNBLOCK, &set, NULL) < 0) { > + printf("sigprocmask() in child failed: %d\n", errno); > + > + exit(1); > + } > + > + pause(); > + > exit(exno); > } > > -static void setup(void) > +static void sigusr_handler(int signum) > { > +} > + > +static void term_handler(int signum) > +{ > + if (pid == 0) { > + printf("Child received signal %d\n", signum); > > + exit(1); > + } else { > + if (pid > 0) > + kill(pid, signum); > + > + tst_brkm(TFAIL, cleanup, "Signal %d received. Exiting", signum); > + } > +} > + > +static void setup(void) > +{ > tst_sig(FORK, DEF_HANDLER, cleanup); > > umask(0); > -- > 1.7.1 > > > ------------------------------------------------------------------------------ > Android is increasing in popularity, but the open development platform that > developers love is also attractive to malware creators. Download this white > paper to learn more about secure code signing practices that can help keep > Android apps secure. > http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk > _______________________________________________ > Ltp-list mailing list > Ltp...@li... > https://lists.sourceforge.net/lists/listinfo/ltp-list > |
From: Stanislav K. <sta...@or...> - 2013-10-31 07:55:02
|
On 10/30/2013 05:45 PM, Jan Stancek wrote: > Hi, > > I think I still see a possible race, try running it with following change: > diff --git a/testcases/kernel/syscalls/setpgid/setpgid03.c b/testcases/kernel/syscalls/setpgid/setpgid03.c > index 34975b7..1d478cb 100644 > --- a/testcases/kernel/syscalls/setpgid/setpgid03.c > +++ b/testcases/kernel/syscalls/setpgid/setpgid03.c > @@ -221,6 +221,7 @@ static void do_child(void) > } > > kill(getppid(), SIGUSR1); > + sleep(1); > > sigemptyset(&set); > sigaddset(&set, SIGUSR1); It seems that this helps: diff --git a/testcases/kernel/syscalls/setpgid/setpgid03.c b/testcases/kernel/syscalls/setpgid/setpgid03.c index 34975b7..e5bf1cd 100644 --- a/testcases/kernel/syscalls/setpgid/setpgid03.c +++ b/testcases/kernel/syscalls/setpgid/setpgid03.c @@ -36,6 +36,7 @@ char *TCID = "setpgid03"; int TST_TOTAL = 1; static pid_t pid = -1; +static volatile int exit_child; static void do_child(void); static void sigusr_handler(int signum); @@ -230,13 +231,15 @@ static void do_child(void) exit(1); } - pause(); + while(!exit_child) { + } exit(exno); } static void sigusr_handler(int signum) { + exit_child = 1; } > I also noticed that test2 can now call setpgid() on process which > can be zombie at that time. It worked fine on my system (RHEL6.4), > but it made me wonder if we can rely on this behavior. If we can, > then also test1's do_child() can be simplified. I tried to do exit() in do_child() before setpgid() was executed in parent and setpgid() returned EPERM. I.e. test1 succeeds in that scenario. But I'm not sure if we can rely on that. I would prefer not to rely... Maybe It would be better to write a standalone program (instead of /bin/kill) which will send SIGUSR1 signal to its parent and then wait to get SIGUSR1 signal from parent and do exit() after that. How do you think? Thank you. > Regards, > Jan > > ----- Original Message ----- >> From: "Stanislav Kholmanskikh" <sta...@or...> >> To: ltp...@li... >> Cc: "vasily isaenko" <vas...@or...> >> Sent: Wednesday, 30 October, 2013 10:07:38 AM >> Subject: [LTP] [PATCH V2 2/2] setpgid03: use SIGUSR1 instead of sleep >> >> I came accross the following rare issue: >> >> setpgid03 1 TPASS : setpgid SUCCESS to set errno to EPERM >> setpgid03 2 TFAIL : setpgid FAILED, expect EACCES got 1 >> >> I suppose It may happen if the test host is under stress. >> >> To prevent this It would be better to use SIGUSR1 for parent-child >> synchronisations. >> >> Signed-off-by: Stanislav Kholmanskikh <sta...@or...> >> --- >> testcases/kernel/syscalls/setpgid/setpgid03.c | 123 >> +++++++++++++++++++++++-- >> 1 files changed, 115 insertions(+), 8 deletions(-) >> >> diff --git a/testcases/kernel/syscalls/setpgid/setpgid03.c >> b/testcases/kernel/syscalls/setpgid/setpgid03.c >> index 2c47bc4..34975b7 100644 >> --- a/testcases/kernel/syscalls/setpgid/setpgid03.c >> +++ b/testcases/kernel/syscalls/setpgid/setpgid03.c >> @@ -1,5 +1,6 @@ >> /* >> * Copyright (c) International Business Machines Corp., 2001 >> + * Copyright (c) 2013 Oracle and/or its affiliates. All Rights Reserved. >> * >> * This program is free software; you can redistribute it and/or modify >> * it under the terms of the GNU General Public License as published by >> @@ -34,15 +35,20 @@ >> char *TCID = "setpgid03"; >> int TST_TOTAL = 1; >> >> +static pid_t pid = -1; >> + >> static void do_child(void); >> +static void sigusr_handler(int signum); >> +static void term_handler(int signum); >> static void setup(void); >> static void cleanup(void); >> >> int main(int ac, char **av) >> { >> - int pid; >> int rval; >> int status; >> + sigset_t sig_set; >> + struct sigaction sa; >> >> int lc; >> char *msg; >> @@ -56,12 +62,42 @@ int main(int ac, char **av) >> >> setup(); >> >> + /* We need to block these signals before fork() >> + * because our children may terminate either >> + * normally (parent receives both SIGUSR1 and SIGCHLD) or >> + * abnormally (parent receives SIGCHLD only) >> + */ >> + sigemptyset(&sig_set); >> + sigaddset(&sig_set, SIGUSR1); >> + sigaddset(&sig_set, SIGCHLD); >> + >> + /* Since we use SIGUSR1 as a control signal It should not >> + * be an "unexpected" one >> + */ >> + memset(&sa, 0, sizeof(sa)); >> + sa.sa_handler = sigusr_handler; >> + if (sigaction(SIGUSR1, &sa, NULL) < 0) >> + tst_brkm(TFAIL | TERRNO, cleanup, "sigaction(SIGUSR1) failed"); >> + >> + /* But for SIGINT, SIGTERM we should kill our children */ >> + sa.sa_handler = term_handler; >> + if (sigaction(SIGTERM, &sa, NULL) < 0) >> + tst_brkm(TFAIL | TERRNO, cleanup, "sigaction(SIGTERM) failed"); >> + if (sigaction(SIGINT, &sa, NULL) < 0) >> + tst_brkm(TFAIL | TERRNO, cleanup, "sigaction(SIGINT) failed"); >> + >> for (lc = 0; TEST_LOOPING(lc); lc++) { >> >> tst_count = 0; >> >> //test1: >> /* sid of the calling process is not same */ >> + >> + if (sigprocmask(SIG_BLOCK, &sig_set, NULL) < 0) >> + tst_brkm(TFAIL | TERRNO, cleanup, >> + "sigprocmask(SIG_BLOCK) in test 1 failed"); >> + >> + >> if ((pid = FORK_OR_VFORK()) == -1) { >> tst_brkm(TBROK, cleanup, "fork() failed"); >> } >> @@ -76,7 +112,14 @@ int main(int ac, char **av) >> #endif >> } >> >> - sleep(1); >> + if (sigwaitinfo(&sig_set, NULL) < 0) >> + tst_brkm(TFAIL | TERRNO, cleanup, >> + "sigwaitinfo() failed"); >> + >> + if (sigprocmask(SIG_UNBLOCK, &sig_set, NULL) < 0) >> + tst_brkm(TFAIL | TERRNO, cleanup, >> + "sigprocmask(SIG_UNBLOCK) in test 2 failed"); >> + >> rval = setpgid(pid, getppid()); >> if (errno == EPERM) { >> tst_resm(TPASS, >> @@ -86,11 +129,14 @@ int main(int ac, char **av) >> "setpgid FAILED, expect %d, return %d", >> EPERM, errno); >> } >> - sleep(1); >> + >> + kill(pid, SIGUSR1); >> >> if (wait(&status) < 0) >> tst_resm(TFAIL | TERRNO, "wait() for child 1 failed"); >> >> + pid = -1; >> + >> if (!(WIFEXITED(status)) || (WEXITSTATUS(status) != 0)) >> tst_resm(TFAIL, "child 1 failed with status %d", >> WEXITSTATUS(status)); >> @@ -100,18 +146,47 @@ int main(int ac, char **av) >> * Value of pid matches the pid of the child process and >> * the child process has exec successfully. Error >> */ >> + >> + if (sigprocmask(SIG_BLOCK, &sig_set, NULL) < 0) >> + tst_brkm(TFAIL | TERRNO, cleanup, >> + "sigprocmask(SIG_BLOCK) in test 2 failed"); >> + >> if ((pid = FORK_OR_VFORK()) == -1) { >> tst_resm(TFAIL, "Fork failed"); >> >> } >> if (pid == 0) { >> - if (execlp("sleep", "sleep", "3", NULL) < 0) { >> - perror("exec failed"); >> + char buf[10]; >> + int ret; >> + >> + ret = snprintf(buf, 10, "%d", getppid()); >> + if ((ret >= 10) || (ret <= 0)) { >> + printf("snprintf() failed\n"); >> + >> + exit(126); >> } >> + >> + const char *args[4 + 1]; >> + args[0] = "kill"; >> + args[1] = "-s"; >> + args[2] = "SIGUSR1"; >> + args[3] = buf; >> + args[4] = NULL; >> + >> + if (execvp(args[0], (char *const *)args) < 0) >> + perror("execvp() failed\n"); >> + >> exit(127); >> } >> >> - sleep(1); >> + if (sigwaitinfo(&sig_set, NULL) < 0) >> + tst_brkm(TFAIL | TERRNO, cleanup, >> + "sigwaitinfo() failed"); >> + >> + if (sigprocmask(SIG_UNBLOCK, &sig_set, NULL) < 0) >> + tst_brkm(TFAIL | TERRNO, cleanup, >> + "sigprocmask(SIG_UNBLOCK) in test 2 failed"); >> + >> rval = setpgid(pid, getppid()); >> if (errno == EACCES) { >> tst_resm(TPASS, >> @@ -124,6 +199,8 @@ int main(int ac, char **av) >> if (wait(&status) < 0) >> tst_resm(TFAIL | TERRNO, "wait() for child 2 failed"); >> >> + pid = -1; >> + >> if (!(WIFEXITED(status)) || (WEXITSTATUS(status) != 0)) >> tst_resm(TFAIL, "child 2 failed with status %d", >> WEXITSTATUS(status)); >> @@ -136,18 +213,48 @@ int main(int ac, char **av) >> static void do_child(void) >> { >> int exno = 0; >> + sigset_t set; >> >> if (setsid() < 0) { >> printf("setsid() failed, errno: %d\n", errno); >> exno = 1; >> } >> - sleep(2); >> + >> + kill(getppid(), SIGUSR1); >> + >> + sigemptyset(&set); >> + sigaddset(&set, SIGUSR1); >> + if (sigprocmask(SIG_UNBLOCK, &set, NULL) < 0) { >> + printf("sigprocmask() in child failed: %d\n", errno); >> + >> + exit(1); >> + } >> + >> + pause(); >> + >> exit(exno); >> } >> >> -static void setup(void) >> +static void sigusr_handler(int signum) >> { >> +} >> + >> +static void term_handler(int signum) >> +{ >> + if (pid == 0) { >> + printf("Child received signal %d\n", signum); >> >> + exit(1); >> + } else { >> + if (pid > 0) >> + kill(pid, signum); >> + >> + tst_brkm(TFAIL, cleanup, "Signal %d received. Exiting", signum); >> + } >> +} >> + >> +static void setup(void) >> +{ >> tst_sig(FORK, DEF_HANDLER, cleanup); >> >> umask(0); >> -- >> 1.7.1 >> >> >> ------------------------------------------------------------------------------ >> Android is increasing in popularity, but the open development platform that >> developers love is also attractive to malware creators. Download this white >> paper to learn more about secure code signing practices that can help keep >> Android apps secure. >> http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk >> _______________________________________________ >> Ltp-list mailing list >> Ltp...@li... >> https://lists.sourceforge.net/lists/listinfo/ltp-list >> |
From: <ch...@su...> - 2013-10-31 15:06:52
|
Hi! There are child/parent synchronization primitives in the LTP library. See include/tst_checkpoint.h and lib/tests/tst_checkpoint_child.c. All that is needed is to initialize checkpoint, call wait in parent and once child is ready call signal in child. That will fix the first race. Once child signals the parent it needs to wait till the parent executes the test, simple pause() in child and kill() from parent should fix this part. -- Cyril Hrubis ch...@su... |
From: Stanislav K. <sta...@or...> - 2013-10-31 15:11:18
|
On 10/31/2013 07:06 PM, ch...@su... wrote: > Hi! > There are child/parent synchronization primitives in the LTP library. > See include/tst_checkpoint.h and lib/tests/tst_checkpoint_child.c. > > All that is needed is to initialize checkpoint, call wait in parent and > once child is ready call signal in child. That will fix the first race. > > Once child signals the parent it needs to wait till the parent executes > the test, simple pause() in child and kill() from parent should fix this > part. Hi! But I need an exec() in the second case. |
From: <ch...@su...> - 2013-10-31 16:30:04
|
Hi! > > But I need an exec() in the second case. I've overlooked the second case, however checkpoint works for this one as well. You need to build a helper binary setpgid03_child.c that would call the signal function and then do the wait. The checkpoint interface is build on the top of unix sockets so it does not matter whether the process was forked, execed or started from termial. All that is needed is correct CWD (which is inherited both on fork and exec) so that the socked special file is found. -- Cyril Hrubis ch...@su... |
From: Stanislav K. <sta...@or...> - 2013-11-07 11:08:00
|
On 10/31/2013 08:29 PM, ch...@su... wrote: > Hi! >> But I need an exec() in the second case. > I've overlooked the second case, however checkpoint works for this one > as well. You need to build a helper binary setpgid03_child.c that would > call the signal function and then do the wait. > > The checkpoint interface is build on the top of unix sockets so it does > not matter whether the process was forked, execed or started from > termial. All that is needed is correct CWD (which is inherited both on > fork and exec) so that the socked special file is found. > There is one issue. Look at lib/tests/tst_checkpoint_parent.c. If the child exits before calling TST_CHECKPOINT_CHILD_WAIT(&checkpoint) (for example, if it gets a signal), the parent will wait at TST_CHECKPOINT_SIGNAL_CHILD(NULL, &checkpoint) (blocking at open(O_WRONLY)) until we kill it manually. I don't think that such situation can occur in this particular test case but if we had more activity (like function calls and some logic) before execution of TST_CHECKPOINT_CHILD_WAIT in the child, we would have troubles. Don't you think so? Or do I miss something? Thank you. |
From: <ch...@su...> - 2013-11-07 11:30:52
|
Hi! > > I've overlooked the second case, however checkpoint works for this one > > as well. You need to build a helper binary setpgid03_child.c that would > > call the signal function and then do the wait. > > > > The checkpoint interface is build on the top of unix sockets so it does > > not matter whether the process was forked, execed or started from > > termial. All that is needed is correct CWD (which is inherited both on > > fork and exec) so that the socked special file is found. > > > There is one issue. > > Look at lib/tests/tst_checkpoint_parent.c. > > If the child exits before calling TST_CHECKPOINT_CHILD_WAIT(&checkpoint) > (for example, if it gets a signal), the parent > will wait at TST_CHECKPOINT_SIGNAL_CHILD(NULL, &checkpoint) (blocking at > open(O_WRONLY)) until we kill it manually. Yes, it will hang. Feel free to send a patch with a timeout. > I don't think that such situation can occur in this particular test case > but if we had more activity (like function calls and some logic) before > execution of TST_CHECKPOINT_CHILD_WAIT in the child, we would have troubles. It's unlikely situation but it may happen. Generally any of the tests may hang (due to bug in libc or kernel for example) and ideally the test driver (ltp-pan currently) should watch for timeouts and kill such tests, which is not implemented currently. I'm working on replacement testdriver when my time allows, I have most of the major parts working and hope to have working prototype soon (hopefully till the end of the year). -- Cyril Hrubis ch...@su... |
From: Stanislav K. <sta...@or...> - 2013-11-13 07:52:56
|
If a child exits before opening TST_CHECKPOINT_FIFO for reading, tst_checkpoint_signal_child() issued from the parent will block forever. To handle such situations added timeout logic to tst_checkpoint_signal_child(); Signed-off-by: Stanislav Kholmanskikh <sta...@or...> --- lib/tst_checkpoint.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 54 insertions(+), 2 deletions(-) diff --git a/lib/tst_checkpoint.c b/lib/tst_checkpoint.c index 56c86b5..a7936da 100644 --- a/lib/tst_checkpoint.c +++ b/lib/tst_checkpoint.c @@ -30,9 +30,54 @@ #include <sys/stat.h> #include <fcntl.h> #include <poll.h> +#include <time.h> #include "tst_checkpoint.h" +/* + * Issue open() on 'path' fifo with O_WRONLY flag and wait for + * a reader up to 'timeout' ms. + * + * Returns: + * 0 - timeout has happened + * > 0 - file descriptor + * -1 - an error has occurred (errno is set accordingly) + * + */ +int open_wronly_timed(const char *path, unsigned int timeout) +{ + int fd; + int interval_ms = 1; /* how often issue open(O_NONBLOCK) */ + long timeleft = timeout; + int saved_errno = errno; + + struct timespec interval; + interval.tv_sec = 0; + interval.tv_nsec = interval_ms * 1000000; /* in ns */ + + for (;;) { + fd = open(path, O_WRONLY | O_NONBLOCK); + if (fd < 0) { + if ((errno == ENXIO) || (errno == EINTR)) { + if (timeleft <= 0) { + errno = saved_errno; + + return 0; + } + + timeleft -= interval_ms; + nanosleep(&interval, NULL); + + continue; + } + + return -1; + } + + return fd; + } +} + void tst_checkpoint_init(const char *file, const int lineno, struct tst_checkpoint *self) { @@ -195,12 +240,19 @@ void tst_checkpoint_signal_child(const char *file, const int lineno, { int ret, fd; - fd = open(TST_CHECKPOINT_FIFO, O_WRONLY); + fd = open_wronly_timed(TST_CHECKPOINT_FIFO, self->timeout); - if (fd < 0) { + switch (fd) { + case 0: + tst_brkm(TBROK, cleanup_fn, + "Checkpoint timeouted after %u msecs at %s:%d", + self->timeout, file, lineno); + break; + case -1: tst_brkm(TBROK | TERRNO, cleanup_fn, "Failed to open fifo '%s' at %s:%d", TST_CHECKPOINT_FIFO, file, lineno); + break; } ret = write(fd, "p", 1); -- 1.7.1 |
From: Jan S. <jst...@re...> - 2013-11-13 08:46:21
|
----- Original Message ----- > From: "Stanislav Kholmanskikh" <sta...@or...> > To: ltp...@li... > Cc: "vasily isaenko" <vas...@or...> > Sent: Wednesday, 13 November, 2013 8:52:42 AM > Subject: [LTP] [RFC PATCH] tst_checkpoint_signal_child: implemented timeout > > If a child exits before opening TST_CHECKPOINT_FIFO > for reading, tst_checkpoint_signal_child() issued from the parent > will block forever. > > To handle such situations added timeout logic to > tst_checkpoint_signal_child(); > > Signed-off-by: Stanislav Kholmanskikh <sta...@or...> > --- > lib/tst_checkpoint.c | 56 > ++++++++++++++++++++++++++++++++++++++++++++++++- > 1 files changed, 54 insertions(+), 2 deletions(-) Hi, I would go with just 2 return values: -1 - for error/timeout fd - otherwise fd == 0 is a valid descriptor we can tell if it was timeout or error by errno code Why do we need "saved_errno"? Regards, Jan > > diff --git a/lib/tst_checkpoint.c b/lib/tst_checkpoint.c > index 56c86b5..a7936da 100644 > --- a/lib/tst_checkpoint.c > +++ b/lib/tst_checkpoint.c > @@ -30,9 +30,54 @@ > #include <sys/stat.h> > #include <fcntl.h> > #include <poll.h> > +#include <time.h> > > #include "tst_checkpoint.h" > > +/* > + * Issue open() on 'path' fifo with O_WRONLY flag and wait for > + * a reader up to 'timeout' ms. > + * > + * Returns: > + * 0 - timeout has happened > + * > 0 - file descriptor > + * -1 - an error has occurred (errno is set accordingly) > + * > + */ > +int open_wronly_timed(const char *path, unsigned int timeout) > +{ > + int fd; > + int interval_ms = 1; /* how often issue open(O_NONBLOCK) */ > + long timeleft = timeout; > + int saved_errno = errno; > + > + struct timespec interval; > + interval.tv_sec = 0; > + interval.tv_nsec = interval_ms * 1000000; /* in ns */ > + > + for (;;) { > + fd = open(path, O_WRONLY | O_NONBLOCK); > + if (fd < 0) { > + if ((errno == ENXIO) || (errno == EINTR)) { > + if (timeleft <= 0) { > + errno = saved_errno; > + > + return 0; > + } > + > + timeleft -= interval_ms; > + nanosleep(&interval, NULL); > + > + continue; > + } > + > + return -1; > + } > + > + return fd; > + } > +} > + > void tst_checkpoint_init(const char *file, const int lineno, > struct tst_checkpoint *self) > { > @@ -195,12 +240,19 @@ void tst_checkpoint_signal_child(const char *file, > const int lineno, > { > int ret, fd; > > - fd = open(TST_CHECKPOINT_FIFO, O_WRONLY); > + fd = open_wronly_timed(TST_CHECKPOINT_FIFO, self->timeout); > > - if (fd < 0) { > + switch (fd) { > + case 0: > + tst_brkm(TBROK, cleanup_fn, > + "Checkpoint timeouted after %u msecs at %s:%d", > + self->timeout, file, lineno); > + break; > + case -1: > tst_brkm(TBROK | TERRNO, cleanup_fn, > "Failed to open fifo '%s' at %s:%d", > TST_CHECKPOINT_FIFO, file, lineno); > + break; > } > > ret = write(fd, "p", 1); > -- > 1.7.1 > > > ------------------------------------------------------------------------------ > DreamFactory - Open Source REST & JSON Services for HTML5 & Native Apps > OAuth, Users, Roles, SQL, NoSQL, BLOB Storage and External API Access > Free app hosting. Or install the open source package on any LAMP server. > Sign up and see examples for AngularJS, jQuery, Sencha Touch and Native! > http://pubads.g.doubleclick.net/gampad/clk?id=63469471&iu=/4140/ostg.clktrk > _______________________________________________ > Ltp-list mailing list > Ltp...@li... > https://lists.sourceforge.net/lists/listinfo/ltp-list > |
From: Stanislav K. <sta...@or...> - 2013-11-13 08:59:06
|
On 11/13/2013 12:46 PM, Jan Stancek wrote: > > ----- Original Message ----- >> From: "Stanislav Kholmanskikh" <sta...@or...> >> To: ltp...@li... >> Cc: "vasily isaenko" <vas...@or...> >> Sent: Wednesday, 13 November, 2013 8:52:42 AM >> Subject: [LTP] [RFC PATCH] tst_checkpoint_signal_child: implemented timeout >> >> If a child exits before opening TST_CHECKPOINT_FIFO >> for reading, tst_checkpoint_signal_child() issued from the parent >> will block forever. >> >> To handle such situations added timeout logic to >> tst_checkpoint_signal_child(); >> >> Signed-off-by: Stanislav Kholmanskikh <sta...@or...> >> --- >> lib/tst_checkpoint.c | 56 >> ++++++++++++++++++++++++++++++++++++++++++++++++- >> 1 files changed, 54 insertions(+), 2 deletions(-) > Hi, > > I would go with just 2 return values: > -1 - for error/timeout > fd - otherwise > > fd == 0 is a valid descriptor > we can tell if it was timeout or error by errno code Hi. Ok. > Why do we need "saved_errno"? Initially I though to use "saved_errno" to handle this situation: * nanosleep() fails and updates global errno * open_wronly_timed() returns But I've looked again on the code and It seems that this situation will not happen and so on "saved_errno" is useless... Thanks. > > Regards, > Jan > >> diff --git a/lib/tst_checkpoint.c b/lib/tst_checkpoint.c >> index 56c86b5..a7936da 100644 >> --- a/lib/tst_checkpoint.c >> +++ b/lib/tst_checkpoint.c >> @@ -30,9 +30,54 @@ >> #include <sys/stat.h> >> #include <fcntl.h> >> #include <poll.h> >> +#include <time.h> >> >> #include "tst_checkpoint.h" >> >> +/* >> + * Issue open() on 'path' fifo with O_WRONLY flag and wait for >> + * a reader up to 'timeout' ms. >> + * >> + * Returns: >> + * 0 - timeout has happened >> + * > 0 - file descriptor >> + * -1 - an error has occurred (errno is set accordingly) >> + * >> + */ >> +int open_wronly_timed(const char *path, unsigned int timeout) >> +{ >> + int fd; >> + int interval_ms = 1; /* how often issue open(O_NONBLOCK) */ >> + long timeleft = timeout; >> + int saved_errno = errno; >> + >> + struct timespec interval; >> + interval.tv_sec = 0; >> + interval.tv_nsec = interval_ms * 1000000; /* in ns */ >> + >> + for (;;) { >> + fd = open(path, O_WRONLY | O_NONBLOCK); >> + if (fd < 0) { >> + if ((errno == ENXIO) || (errno == EINTR)) { >> + if (timeleft <= 0) { >> + errno = saved_errno; >> + >> + return 0; >> + } >> + >> + timeleft -= interval_ms; >> + nanosleep(&interval, NULL); >> + >> + continue; >> + } >> + >> + return -1; >> + } >> + >> + return fd; >> + } >> +} >> + >> void tst_checkpoint_init(const char *file, const int lineno, >> struct tst_checkpoint *self) >> { >> @@ -195,12 +240,19 @@ void tst_checkpoint_signal_child(const char *file, >> const int lineno, >> { >> int ret, fd; >> >> - fd = open(TST_CHECKPOINT_FIFO, O_WRONLY); >> + fd = open_wronly_timed(TST_CHECKPOINT_FIFO, self->timeout); >> >> - if (fd < 0) { >> + switch (fd) { >> + case 0: >> + tst_brkm(TBROK, cleanup_fn, >> + "Checkpoint timeouted after %u msecs at %s:%d", >> + self->timeout, file, lineno); >> + break; >> + case -1: >> tst_brkm(TBROK | TERRNO, cleanup_fn, >> "Failed to open fifo '%s' at %s:%d", >> TST_CHECKPOINT_FIFO, file, lineno); >> + break; >> } >> >> ret = write(fd, "p", 1); >> -- >> 1.7.1 >> >> >> ------------------------------------------------------------------------------ >> DreamFactory - Open Source REST & JSON Services for HTML5 & Native Apps >> OAuth, Users, Roles, SQL, NoSQL, BLOB Storage and External API Access >> Free app hosting. Or install the open source package on any LAMP server. >> Sign up and see examples for AngularJS, jQuery, Sencha Touch and Native! >> http://pubads.g.doubleclick.net/gampad/clk?id=63469471&iu=/4140/ostg.clktrk >> _______________________________________________ >> Ltp-list mailing list >> Ltp...@li... >> https://lists.sourceforge.net/lists/listinfo/ltp-list >> |
From: Stanislav K. <sta...@or...> - 2013-11-13 09:15:29
|
If a child exits before opening TST_CHECKPOINT_FIFO for reading, tst_checkpoint_signal_child() issued from the parent will block forever. To handle such situations added timeout logic to tst_checkpoint_signal_child(); Signed-off-by: Stanislav Kholmanskikh <sta...@or...> --- lib/tst_checkpoint.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 53 insertions(+), 4 deletions(-) diff --git a/lib/tst_checkpoint.c b/lib/tst_checkpoint.c index 56c86b5..842ac74 100644 --- a/lib/tst_checkpoint.c +++ b/lib/tst_checkpoint.c @@ -30,9 +30,52 @@ #include <sys/stat.h> #include <fcntl.h> #include <poll.h> +#include <time.h> #include "tst_checkpoint.h" +/* + * Issue open() on 'path' fifo with O_WRONLY flag and wait for + * a reader up to 'timeout' ms. + * + * Returns: + * > 0 - file descriptor + * -1 - an error has occurred (errno is set accordingly) + * + */ +int open_wronly_timed(const char *path, unsigned int timeout) +{ + int fd; + int interval_ms = 1; /* how often issue open(O_NONBLOCK) */ + long timeleft = timeout; + + struct timespec interval; + interval.tv_sec = 0; + interval.tv_nsec = interval_ms * 1000000; /* in ns */ + + for (;;) { + fd = open(path, O_WRONLY | O_NONBLOCK); + if (fd < 0) { + if ((errno == ENXIO) || (errno == EINTR)) { + if (timeleft <= 0) { + errno = ETIMEDOUT; + + return -1; + } + + timeleft -= interval_ms; + nanosleep(&interval, NULL); + + continue; + } + + return -1; + } + + return fd; + } +} + void tst_checkpoint_init(const char *file, const int lineno, struct tst_checkpoint *self) { @@ -195,12 +238,18 @@ void tst_checkpoint_signal_child(const char *file, const int lineno, { int ret, fd; - fd = open(TST_CHECKPOINT_FIFO, O_WRONLY); + fd = open_wronly_timed(TST_CHECKPOINT_FIFO, self->timeout); if (fd < 0) { - tst_brkm(TBROK | TERRNO, cleanup_fn, - "Failed to open fifo '%s' at %s:%d", - TST_CHECKPOINT_FIFO, file, lineno); + if (errno == ETIMEDOUT) { + tst_brkm(TBROK, cleanup_fn, + "Checkpoint timeouted after %u msecs at %s:%d", + self->timeout, file, lineno); + } else { + tst_brkm(TBROK | TERRNO, cleanup_fn, + "Failed to open fifo '%s' at %s:%d", + TST_CHECKPOINT_FIFO, file, lineno); + } } ret = write(fd, "p", 1); -- 1.7.1 |
From: <ch...@su...> - 2013-11-13 17:50:22
|
Hi! > If a child exits before opening TST_CHECKPOINT_FIFO > for reading, tst_checkpoint_signal_child() issued from the parent > will block forever. > > To handle such situations added timeout logic to tst_checkpoint_signal_child(); > > Signed-off-by: Stanislav Kholmanskikh <sta...@or...> > --- > lib/tst_checkpoint.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++--- > 1 files changed, 53 insertions(+), 4 deletions(-) > > diff --git a/lib/tst_checkpoint.c b/lib/tst_checkpoint.c > index 56c86b5..842ac74 100644 > --- a/lib/tst_checkpoint.c > +++ b/lib/tst_checkpoint.c > @@ -30,9 +30,52 @@ > #include <sys/stat.h> > #include <fcntl.h> > #include <poll.h> > +#include <time.h> > > #include "tst_checkpoint.h" > > +/* > + * Issue open() on 'path' fifo with O_WRONLY flag and wait for > + * a reader up to 'timeout' ms. > + * > + * Returns: > + * > 0 - file descriptor > + * -1 - an error has occurred (errno is set accordingly) > + * > + */ > +int open_wronly_timed(const char *path, unsigned int timeout) > +{ > + int fd; > + int interval_ms = 1; /* how often issue open(O_NONBLOCK) */ > + long timeleft = timeout; > + > + struct timespec interval; > + interval.tv_sec = 0; > + interval.tv_nsec = interval_ms * 1000000; /* in ns */ > + > + for (;;) { What about using for (i = 0; i < timeout; i += interval) { > + fd = open(path, O_WRONLY | O_NONBLOCK); > + if (fd < 0) { > + if ((errno == ENXIO) || (errno == EINTR)) { > + if (timeleft <= 0) { > + errno = ETIMEDOUT; > + > + return -1; > + } > + > + timeleft -= interval_ms; > + nanosleep(&interval, NULL); What about using usleep() which has simplier interface instead? > + continue; > + } > + > + return -1; > + } > + > + return fd; > + } > +} > + > void tst_checkpoint_init(const char *file, const int lineno, > struct tst_checkpoint *self) > { > @@ -195,12 +238,18 @@ void tst_checkpoint_signal_child(const char *file, const int lineno, > { > int ret, fd; > > - fd = open(TST_CHECKPOINT_FIFO, O_WRONLY); > + fd = open_wronly_timed(TST_CHECKPOINT_FIFO, self->timeout); > > if (fd < 0) { > - tst_brkm(TBROK | TERRNO, cleanup_fn, > - "Failed to open fifo '%s' at %s:%d", > - TST_CHECKPOINT_FIFO, file, lineno); > + if (errno == ETIMEDOUT) { > + tst_brkm(TBROK, cleanup_fn, > + "Checkpoint timeouted after %u msecs at %s:%d", > + self->timeout, file, lineno); > + } else { > + tst_brkm(TBROK | TERRNO, cleanup_fn, > + "Failed to open fifo '%s' at %s:%d", > + TST_CHECKPOINT_FIFO, file, lineno); > + } Hmm, there is no need to complicate this part, if you have set the errno to ETIMEOUTED the tst_brkm() will include the errno and will look like: foo01 1 TBROK : Failed to open fifo 'tst_checkpoint_fifo' at tst_checkopoint.c:211: ERRNO=ETIMEDOUT(110): Connection timed out Which is in my opinion descriptive enough. But we need to add EDTIMEDOUT into the tst_res.c fist, actually there are still some errno values missing, so I will add them them there beforehand. diff --git a/lib/tst_res.c b/lib/tst_res.c index f73022b..7d87492 100644 --- a/lib/tst_res.c +++ b/lib/tst_res.c @@ -255,6 +255,7 @@ static const char *strerrnodef(int err) PAIR(ENOSYS) PAIR(ENOTEMPTY) PAIR(ELOOP) + PAIR(ETIMEDOUT) }; return pair_lookup(errno_pairs, err); } -- Cyril Hrubis ch...@su... |
From: Stanislav K. <sta...@or...> - 2013-11-14 06:38:11
|
Signed-off-by: Stanislav Kholmanskikh <sta...@or...> --- lib/tst_res.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/lib/tst_res.c b/lib/tst_res.c index f73022b..698b12c 100644 --- a/lib/tst_res.c +++ b/lib/tst_res.c @@ -255,6 +255,8 @@ static const char *strerrnodef(int err) PAIR(ENOSYS) PAIR(ENOTEMPTY) PAIR(ELOOP) + PAIR(ETIMEDOUT) + PAIR(ETIME) }; return pair_lookup(errno_pairs, err); } -- 1.7.1 |
From: <ch...@su...> - 2013-11-14 14:20:07
|
Hi! > Signed-off-by: Stanislav Kholmanskikh <sta...@or...> > --- > lib/tst_res.c | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/lib/tst_res.c b/lib/tst_res.c > index f73022b..698b12c 100644 > --- a/lib/tst_res.c > +++ b/lib/tst_res.c > @@ -255,6 +255,8 @@ static const char *strerrnodef(int err) > PAIR(ENOSYS) > PAIR(ENOTEMPTY) > PAIR(ELOOP) > + PAIR(ETIMEDOUT) > + PAIR(ETIME) > }; > return pair_lookup(errno_pairs, err); > } I've just pushed patch that add all ERRNO values found in headers on recent enough distribution. Please check that it compiles at older distros you are using and if not send a patch with ifdefs as they are around the HWPOISON. -- Cyril Hrubis ch...@su... |