[RFC,v4,3/5] support record failure: flush stdout, use _exit ()

Message ID 20181121183936.8176-3-mathieu.desnoyers@efficios.com
State New
Headers show
Series
  • [RFC,v4,1/5] glibc: Perform rseq(2) registration at nptl init and thread creation
Related show

Commit Message

Mathieu Desnoyers Nov. 21, 2018, 6:39 p.m.
Using "exit ()" from pthread_atfork handlers hangs the process. It is
therefore not a good way to exit when reporting a testing error.

Use _exit () instead, which directly exits the process. However,
considering that the buffered stdout is used to output test failure
messages, we first need to flush stdout before exiting.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

CC: Carlos O'Donell <carlos@redhat.com>
CC: Florian Weimer <fweimer@redhat.com>
CC: Joseph Myers <joseph@codesourcery.com>
CC: Szabolcs Nagy <szabolcs.nagy@arm.com>
CC: libc-alpha@sourceware.org
---
 support/check.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

-- 
2.17.1

Comments

Florian Weimer Nov. 21, 2018, 6:50 p.m. | #1
* Mathieu Desnoyers:

> Using "exit ()" from pthread_atfork handlers hangs the process. It is

> therefore not a good way to exit when reporting a testing error.

>

> Use _exit () instead, which directly exits the process. However,

> considering that the buffered stdout is used to output test failure

> messages, we first need to flush stdout before exiting.


This is not correct; we need the atexit handlers for cleaning up
temporary files.

It should be possible to call support_record_failure and rely on the
shared memory segment to register the test failure, so that it is
eventually reflected in the exit state (even if the failure happens in
the subprocess).

Thanks,
Florian
Mathieu Desnoyers Nov. 21, 2018, 6:52 p.m. | #2
----- On Nov 21, 2018, at 1:50 PM, Florian Weimer fweimer@redhat.com wrote:

> * Mathieu Desnoyers:

> 

>> Using "exit ()" from pthread_atfork handlers hangs the process. It is

>> therefore not a good way to exit when reporting a testing error.

>>

>> Use _exit () instead, which directly exits the process. However,

>> considering that the buffered stdout is used to output test failure

>> messages, we first need to flush stdout before exiting.

> 

> This is not correct; we need the atexit handlers for cleaning up

> temporary files.

> 

> It should be possible to call support_record_failure and rely on the

> shared memory segment to register the test failure, so that it is

> eventually reflected in the exit state (even if the failure happens in

> the subprocess).


Calling exit() from a pthread_atfork handler unfortunately seems to hang
the process :-/

What would you recommend to deal with that situation ?

Thanks,

Mathieu

> 

> Thanks,

> Florian


-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
Mathieu Desnoyers Nov. 21, 2018, 6:55 p.m. | #3
----- On Nov 21, 2018, at 1:52 PM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote:

> ----- On Nov 21, 2018, at 1:50 PM, Florian Weimer fweimer@redhat.com wrote:

> 

>> * Mathieu Desnoyers:

>> 

>>> Using "exit ()" from pthread_atfork handlers hangs the process. It is

>>> therefore not a good way to exit when reporting a testing error.

>>>

>>> Use _exit () instead, which directly exits the process. However,

>>> considering that the buffered stdout is used to output test failure

>>> messages, we first need to flush stdout before exiting.

>> 

>> This is not correct; we need the atexit handlers for cleaning up

>> temporary files.

>> 

>> It should be possible to call support_record_failure and rely on the

>> shared memory segment to register the test failure, so that it is

>> eventually reflected in the exit state (even if the failure happens in

>> the subprocess).

> 

> Calling exit() from a pthread_atfork handler unfortunately seems to hang

> the process :-/

> 

> What would you recommend to deal with that situation ?


Or do you mean _not_ exiting from the pthread_atfork handlers, but just
record the error there, and continue normally, then catch the error in
the parent ?

Thanks,

Mathieu

> 

> Thanks,

> 

> Mathieu

> 

>> 

>> Thanks,

>> Florian

> 

> --

> Mathieu Desnoyers

> EfficiOS Inc.

> http://www.efficios.com


-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
Florian Weimer Nov. 21, 2018, 6:55 p.m. | #4
* Mathieu Desnoyers:

> ----- On Nov 21, 2018, at 1:50 PM, Florian Weimer fweimer@redhat.com wrote:

>

>> * Mathieu Desnoyers:

>> 

>>> Using "exit ()" from pthread_atfork handlers hangs the process. It is

>>> therefore not a good way to exit when reporting a testing error.

>>>

>>> Use _exit () instead, which directly exits the process. However,

>>> considering that the buffered stdout is used to output test failure

>>> messages, we first need to flush stdout before exiting.

>> 

>> This is not correct; we need the atexit handlers for cleaning up

>> temporary files.

>> 

>> It should be possible to call support_record_failure and rely on the

>> shared memory segment to register the test failure, so that it is

>> eventually reflected in the exit state (even if the failure happens in

>> the subprocess).

>

> Calling exit() from a pthread_atfork handler unfortunately seems to hang

> the process :-/

>

> What would you recommend to deal with that situation ?


Why do you want to terminate the process there?  Is this part of the
test?

Thanks,
Florian
Florian Weimer Nov. 21, 2018, 6:56 p.m. | #5
* Mathieu Desnoyers:

> ----- On Nov 21, 2018, at 1:52 PM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote:

>

>> ----- On Nov 21, 2018, at 1:50 PM, Florian Weimer fweimer@redhat.com wrote:

>> 

>>> * Mathieu Desnoyers:

>>> 

>>>> Using "exit ()" from pthread_atfork handlers hangs the process. It is

>>>> therefore not a good way to exit when reporting a testing error.

>>>>

>>>> Use _exit () instead, which directly exits the process. However,

>>>> considering that the buffered stdout is used to output test failure

>>>> messages, we first need to flush stdout before exiting.

>>> 

>>> This is not correct; we need the atexit handlers for cleaning up

>>> temporary files.

>>> 

>>> It should be possible to call support_record_failure and rely on the

>>> shared memory segment to register the test failure, so that it is

>>> eventually reflected in the exit state (even if the failure happens in

>>> the subprocess).

>> 

>> Calling exit() from a pthread_atfork handler unfortunately seems to hang

>> the process :-/

>> 

>> What would you recommend to deal with that situation ?

>

> Or do you mean _not_ exiting from the pthread_atfork handlers, but just

> record the error there, and continue normally, then catch the error in

> the parent ?


Yes, support_record_failure is for delayed failure reporting.

Thanks,
Florian
Mathieu Desnoyers Nov. 21, 2018, 10:23 p.m. | #6
----- On Nov 21, 2018, at 1:56 PM, Florian Weimer fweimer@redhat.com wrote:

> * Mathieu Desnoyers:

> 

>> ----- On Nov 21, 2018, at 1:52 PM, Mathieu Desnoyers

>> mathieu.desnoyers@efficios.com wrote:

>>

>>> ----- On Nov 21, 2018, at 1:50 PM, Florian Weimer fweimer@redhat.com wrote:

>>> 

>>>> * Mathieu Desnoyers:

>>>> 

>>>>> Using "exit ()" from pthread_atfork handlers hangs the process. It is

>>>>> therefore not a good way to exit when reporting a testing error.

>>>>>

>>>>> Use _exit () instead, which directly exits the process. However,

>>>>> considering that the buffered stdout is used to output test failure

>>>>> messages, we first need to flush stdout before exiting.

>>>> 

>>>> This is not correct; we need the atexit handlers for cleaning up

>>>> temporary files.

>>>> 

>>>> It should be possible to call support_record_failure and rely on the

>>>> shared memory segment to register the test failure, so that it is

>>>> eventually reflected in the exit state (even if the failure happens in

>>>> the subprocess).

>>> 

>>> Calling exit() from a pthread_atfork handler unfortunately seems to hang

>>> the process :-/

>>> 

>>> What would you recommend to deal with that situation ?

>>

>> Or do you mean _not_ exiting from the pthread_atfork handlers, but just

>> record the error there, and continue normally, then catch the error in

>> the parent ?

> 

> Yes, support_record_failure is for delayed failure reporting.


Good. I'll favor returning errors back to do_test () whenever possible.

When it's not, I'll use delayed failure reporting rather that FAIL_EXIT1 ()
when the test needs to report an error and has not way to return it to
do_test ().

I'll keep FAIL_EXIT1 for "setup" errors that are outside of the scope of
what the test is actually testing, and also for execution contexts running
after main exits (atexit handler, destructors, pthread key destroy,
__call_tls_dtors.

Thanks,

Mathieu


-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

Patch

diff --git a/support/check.c b/support/check.c
index 78f2b3cde1..bed1d20457 100644
--- a/support/check.c
+++ b/support/check.c
@@ -22,6 +22,7 @@ 
 #include <stdarg.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <unistd.h>
 #include <support/test-driver.h>
 
 static void
@@ -56,5 +57,6 @@  support_exit_failure_impl (int status, const char *file, int line,
   va_start (ap, format);
   print_failure (file, line, format, ap);
   va_end (ap);
-  exit (status);
+  fflush (stdout);
+  _exit (status);
 }