201 lines
7.8 KiB
Diff
201 lines
7.8 KiB
Diff
commit f0419e6a10740a672b28e112c409ae24f5e890ab
|
|
Author: Jakub Jelinek <jakub@redhat.com>
|
|
Date: Thu Mar 4 15:15:33 2021 +0100
|
|
|
|
[PATCH] pthread_once hangs when init routine throws an exception [BZ #18435]
|
|
|
|
This is another attempt at making pthread_once handle throwing exceptions
|
|
from the init routine callback. As the new testcases show, just switching
|
|
to the cleanup attribute based cleanup does fix the tst-once5 test, but
|
|
breaks the new tst-oncey3 test. That is because when throwing exceptions,
|
|
only the unwind info registered cleanups (i.e. C++ destructors or cleanup
|
|
attribute), when cancelling threads and there has been unwind info from the
|
|
cancellation point up to whatever needs cleanup both unwind info registered
|
|
cleanups and THREAD_SETMEM (self, cleanup, ...) registered cleanups are
|
|
invoked, but once we hit some frame with no unwind info, only the
|
|
THREAD_SETMEM (self, cleanup, ...) registered cleanups are invoked.
|
|
So, to stay fully backwards compatible (allow init routines without
|
|
unwind info which encounter cancellation points) and handle exception throwing
|
|
we actually need to register the pthread_once cleanups in both unwind info
|
|
and in the THREAD_SETMEM (self, cleanup, ...) way.
|
|
If an exception is thrown, only the former will happen and we in that case
|
|
need to also unregister the THREAD_SETMEM (self, cleanup, ...) registered
|
|
handler, because otherwise after catching the exception the user code could
|
|
call deeper into the stack some cancellation point, get cancelled and then
|
|
a stale cleanup handler would clobber stack and probably crash.
|
|
If a thread calling init routine is cancelled and unwind info ends before
|
|
the pthread_once frame, it will be cleaned up through self->cleanup as
|
|
before. And if unwind info is present, unwind_stop first calls the
|
|
self->cleanup registered handler for the frame, then it will call the
|
|
unwind info registered handler but that will already see __do_it == 0
|
|
and do nothing.
|
|
|
|
# Conflicts:
|
|
# nptl/Makefile
|
|
# (The usual cleanups because they don't match.)
|
|
# sysdeps/pthread/Makefile
|
|
# (The usual cleanups because all the other tests aren't moved.)
|
|
|
|
diff --git a/nptl/Makefile b/nptl/Makefile
|
|
index dcf3868869767015..70a3be23ecfcd9c9 100644
|
|
--- a/nptl/Makefile
|
|
+++ b/nptl/Makefile
|
|
@@ -334,10 +334,6 @@ xtests = tst-setuid1 tst-setuid1-static tst-setuid2 \
|
|
tst-mutexpp1 tst-mutexpp6 tst-mutexpp10
|
|
test-srcs = tst-oddstacklimit
|
|
|
|
-# Test expected to fail on most targets (except x86_64) due to bug
|
|
-# 18435 - pthread_once hangs when init routine throws an exception.
|
|
-test-xfail-tst-once5 = yes
|
|
-
|
|
# Files which must not be linked with libpthread.
|
|
tests-nolibpthread = tst-unload
|
|
|
|
diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
|
|
index a2d48b2015cd385c..7ddc166cf32414c4 100644
|
|
--- a/nptl/pthreadP.h
|
|
+++ b/nptl/pthreadP.h
|
|
@@ -571,6 +571,67 @@ extern void __pthread_cleanup_pop (struct _pthread_cleanup_buffer *buffer,
|
|
# undef pthread_cleanup_pop
|
|
# define pthread_cleanup_pop(execute) \
|
|
__pthread_cleanup_pop (&_buffer, (execute)); }
|
|
+
|
|
+# if defined __EXCEPTIONS && !defined __cplusplus
|
|
+/* Structure to hold the cleanup handler information. */
|
|
+struct __pthread_cleanup_combined_frame
|
|
+{
|
|
+ void (*__cancel_routine) (void *);
|
|
+ void *__cancel_arg;
|
|
+ int __do_it;
|
|
+ struct _pthread_cleanup_buffer __buffer;
|
|
+};
|
|
+
|
|
+/* Special cleanup macros which register cleanup both using
|
|
+ __pthread_cleanup_{push,pop} and using cleanup attribute. This is needed
|
|
+ for pthread_once, so that it supports both throwing exceptions from the
|
|
+ pthread_once callback (only cleanup attribute works there) and cancellation
|
|
+ of the thread running the callback if the callback or some routines it
|
|
+ calls don't have unwind information. */
|
|
+
|
|
+static __always_inline void
|
|
+__pthread_cleanup_combined_routine (struct __pthread_cleanup_combined_frame
|
|
+ *__frame)
|
|
+{
|
|
+ if (__frame->__do_it)
|
|
+ {
|
|
+ __frame->__cancel_routine (__frame->__cancel_arg);
|
|
+ __frame->__do_it = 0;
|
|
+ __pthread_cleanup_pop (&__frame->__buffer, 0);
|
|
+ }
|
|
+}
|
|
+
|
|
+static inline void
|
|
+__pthread_cleanup_combined_routine_voidptr (void *__arg)
|
|
+{
|
|
+ struct __pthread_cleanup_combined_frame *__frame
|
|
+ = (struct __pthread_cleanup_combined_frame *) __arg;
|
|
+ if (__frame->__do_it)
|
|
+ {
|
|
+ __frame->__cancel_routine (__frame->__cancel_arg);
|
|
+ __frame->__do_it = 0;
|
|
+ }
|
|
+}
|
|
+
|
|
+# define pthread_cleanup_combined_push(routine, arg) \
|
|
+ do { \
|
|
+ void (*__cancel_routine) (void *) = (routine); \
|
|
+ struct __pthread_cleanup_combined_frame __clframe \
|
|
+ __attribute__ ((__cleanup__ (__pthread_cleanup_combined_routine))) \
|
|
+ = { .__cancel_routine = __cancel_routine, .__cancel_arg = (arg), \
|
|
+ .__do_it = 1 }; \
|
|
+ __pthread_cleanup_push (&__clframe.__buffer, \
|
|
+ __pthread_cleanup_combined_routine_voidptr, \
|
|
+ &__clframe);
|
|
+
|
|
+# define pthread_cleanup_combined_pop(execute) \
|
|
+ __pthread_cleanup_pop (&__clframe.__buffer, 0); \
|
|
+ __clframe.__do_it = 0; \
|
|
+ if (execute) \
|
|
+ __cancel_routine (__clframe.__cancel_arg); \
|
|
+ } while (0)
|
|
+
|
|
+# endif
|
|
#endif
|
|
|
|
extern void __pthread_cleanup_push_defer (struct _pthread_cleanup_buffer *buffer,
|
|
diff --git a/nptl/pthread_once.c b/nptl/pthread_once.c
|
|
index 1653226286dc3539..45e965e8743d9412 100644
|
|
--- a/nptl/pthread_once.c
|
|
+++ b/nptl/pthread_once.c
|
|
@@ -111,11 +111,11 @@ __pthread_once_slow (pthread_once_t *once_control, void (*init_routine) (void))
|
|
/* This thread is the first here. Do the initialization.
|
|
Register a cleanup handler so that in case the thread gets
|
|
interrupted the initialization can be restarted. */
|
|
- pthread_cleanup_push (clear_once_control, once_control);
|
|
+ pthread_cleanup_combined_push (clear_once_control, once_control);
|
|
|
|
init_routine ();
|
|
|
|
- pthread_cleanup_pop (0);
|
|
+ pthread_cleanup_combined_pop (0);
|
|
|
|
|
|
/* Mark *once_control as having finished the initialization. We need
|
|
diff --git a/nptl/tst-once5.cc b/nptl/tst-once5.cc
|
|
index d232266c3ace89d9..dda18e610c9114bc 100644
|
|
--- a/nptl/tst-once5.cc
|
|
+++ b/nptl/tst-once5.cc
|
|
@@ -59,7 +59,7 @@ do_test (void)
|
|
" throwing an exception", stderr);
|
|
}
|
|
catch (OnceException) {
|
|
- if (1 < niter)
|
|
+ if (niter > 1)
|
|
fputs ("pthread_once unexpectedly threw", stderr);
|
|
result = 0;
|
|
}
|
|
@@ -75,7 +75,5 @@ do_test (void)
|
|
return result;
|
|
}
|
|
|
|
-// The test currently hangs and is XFAILed. Reduce the timeout.
|
|
-#define TIMEOUT 1
|
|
#define TEST_FUNCTION do_test ()
|
|
#include "../test-skeleton.c"
|
|
diff --git a/sysdeps/pthread/Makefile b/sysdeps/pthread/Makefile
|
|
index 14ef04247cb84ad3..80a71f3f9f0e72ae 100644
|
|
--- a/sysdeps/pthread/Makefile
|
|
+++ b/sysdeps/pthread/Makefile
|
|
@@ -35,7 +35,7 @@ tst-create1mod.so-no-z-defs = yes
|
|
|
|
tests += tst-once1 tst-once2 tst-once3 tst-once4
|
|
|
|
-tests += tst-oncex3 tst-oncex4
|
|
+tests += tst-oncex3 tst-oncex4 tst-oncey3 tst-oncey4
|
|
|
|
ifeq ($(build-shared),yes)
|
|
# Build all the modules even when not actually running test programs.
|
|
@@ -44,6 +44,8 @@ endif
|
|
|
|
CFLAGS-tst-oncex3.c += -fexceptions
|
|
CFLAGS-tst-oncex4.c += -fexceptions
|
|
+CFLAGS-tst-oncey3.c += -fno-exceptions -fno-asynchronous-unwind-tables
|
|
+CFLAGS-tst-oncey4.c += -fno-exceptions -fno-asynchronous-unwind-tables
|
|
|
|
modules-names += tst-create1mod
|
|
test-modules = $(addprefix $(objpfx),$(addsuffix .so,$(modules-names)))
|
|
diff --git a/sysdeps/pthread/tst-oncey3.c b/sysdeps/pthread/tst-oncey3.c
|
|
new file mode 100644
|
|
index 0000000000000000..08225b88dc06b979
|
|
--- /dev/null
|
|
+++ b/sysdeps/pthread/tst-oncey3.c
|
|
@@ -0,0 +1 @@
|
|
+#include "tst-once3.c"
|
|
diff --git a/sysdeps/pthread/tst-oncey4.c b/sysdeps/pthread/tst-oncey4.c
|
|
new file mode 100644
|
|
index 0000000000000000..9b4d98f3f13c265a
|
|
--- /dev/null
|
|
+++ b/sysdeps/pthread/tst-oncey4.c
|
|
@@ -0,0 +1 @@
|
|
+#include "tst-once4.c"
|