[i-g-t,7/9] lib: Make sure we leak no child processes

Submitted by Daniel Vetter on Feb. 11, 2019, 6:02 p.m.

Details

Message ID 20190211180208.29406-7-daniel.vetter@ffwll.ch
State New
Headers show
Series "Series without cover letter" ( rev: 4 3 2 1 ) in IGT

Not browsing as part of any series.

Commit Message

Daniel Vetter Feb. 11, 2019, 6:02 p.m.
There's a lot more ways to leak children than igt_fork, some even
handrolled. So check for that. Also have a nice littel testcase for
that too.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 lib/igt_core.c       |  4 ++++
 lib/tests/igt_fork.c | 18 ++++++++++++++++++
 2 files changed, 22 insertions(+)

Patch hide | download patch | download mbox

diff --git a/lib/igt_core.c b/lib/igt_core.c
index 3053697da58c..f0e186244bc3 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -1492,6 +1492,8 @@  void __igt_fail_assert(const char *domain, const char *file, const int line,
  */
 void igt_exit(void)
 {
+	int tmp;
+
 	igt_exit_called = true;
 
 	if (igt_key_file)
@@ -1527,6 +1529,8 @@  void igt_exit(void)
 		kill(test_children[c], SIGKILL);
 	assert(!num_test_children);
 
+	assert(wait(&tmp) == -1 && errno == ECHILD);
+
 	if (!test_with_subtests) {
 		struct timespec now;
 		const char *result;
diff --git a/lib/tests/igt_fork.c b/lib/tests/igt_fork.c
index e5b0ab016b23..b486d07000bb 100644
--- a/lib/tests/igt_fork.c
+++ b/lib/tests/igt_fork.c
@@ -68,6 +68,20 @@  static void igt_fork_leak(void)
 	}
 }
 
+static void plain_fork_leak(void)
+{
+	int pid;
+
+	switch (pid = fork()) {
+	case -1:
+		internal_assert(0);
+	case 0:
+		sleep(1);
+	default:
+		exit(0);
+	}
+}
+
 static void igt_fork_timeout_leak(void)
 {
 	igt_fork(i, 1) {
@@ -122,4 +136,8 @@  int main(int argc, char **argv)
 	/* check that igt_waitchildren_timeout cleans up*/
 	ret = do_fork(igt_fork_timeout_leak);
 	internal_assert(ret == IGT_EXIT_FAILURE);
+
+	/* check that any other process leaks are caught*/
+	ret = do_fork(plain_fork_leak);
+	internal_assert(ret == SIGABRT + 128);
 }

Comments

Quoting Daniel Vetter (2019-02-11 18:02:06)
> @@ -1527,6 +1529,8 @@ void igt_exit(void)
>                 kill(test_children[c], SIGKILL);
>         assert(!num_test_children);
>  
> +       assert(wait(&tmp) == -1 && errno == ECHILD);

Turning a previous failedsafe "bug" into an infinite wait.
assert(waitpid(-1, &tmp, WNOHANG) == -1 && errno == ECHILD);
-Chris
Quoting Daniel Vetter (2019-02-11 22:43:15)
> There's a lot more ways to leak children than igt_fork, some even
> handrolled. So check for that. Also have a nice littel testcase for
> that too.
> 
> v2: Don't hang if there's a leaked child process (Chris). Has the
> added benefit that my library unit test also gets faster!
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

> ---
>  lib/igt_core.c       |  4 ++++
>  lib/tests/igt_fork.c | 18 ++++++++++++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 3053697da58c..d2cfc8e6da20 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -1492,6 +1492,8 @@ void __igt_fail_assert(const char *domain, const char *file, const int line,
>   */
>  void igt_exit(void)
>  {
> +       int tmp;
> +
>         igt_exit_called = true;
>  
>         if (igt_key_file)
> @@ -1527,6 +1529,8 @@ void igt_exit(void)
>                 kill(test_children[c], SIGKILL);
>         assert(!num_test_children);
>  
> +       assert(waitpid(-1, &tmp, WNOHANG) == -1 && errno == ECHILD);
> +
>         if (!test_with_subtests) {
>                 struct timespec now;
>                 const char *result;
> diff --git a/lib/tests/igt_fork.c b/lib/tests/igt_fork.c
> index e5b0ab016b23..b486d07000bb 100644
> --- a/lib/tests/igt_fork.c
> +++ b/lib/tests/igt_fork.c
> @@ -68,6 +68,20 @@ static void igt_fork_leak(void)
>         }
>  }
>  
> +static void plain_fork_leak(void)
> +{
> +       int pid;
> +
> +       switch (pid = fork()) {
> +       case -1:
> +               internal_assert(0);
> +       case 0:
> +               sleep(1);
> +       default:
> +               exit(0);
> +       }
> +}
> +
>  static void igt_fork_timeout_leak(void)
>  {
>         igt_fork(i, 1) {
> @@ -122,4 +136,8 @@ int main(int argc, char **argv)
>         /* check that igt_waitchildren_timeout cleans up*/
>         ret = do_fork(igt_fork_timeout_leak);
>         internal_assert(ret == IGT_EXIT_FAILURE);
> +
> +       /* check that any other process leaks are caught*/
> +       ret = do_fork(plain_fork_leak);
> +       internal_assert(ret == SIGABRT + 128);

I'm mulling over
internal_assert(signaled(status, SIGABRT));

static bool signaled(int status, int sig)
{
	return WIFSIGNALED(status) && WTERMSIG(status) == sig;
}

static bool exited(int status, int code)
{
	return WIFEXITED(status) && WEXITSTATUS(status) == code;
}
-Chris