Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions Include/internal/pycore_thread.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#ifndef Py_INTERNAL_THREAD_H
#define Py_INTERNAL_THREAD_H

#ifdef __cplusplus
extern "C" {
#endif

#include <stdint.h>

#ifndef Py_BUILD_CORE
# error "this header requires Py_BUILD_CORE define"
#endif

/*
* Find the size of the stack the system uses by default.
*/
PyAPI_FUNC(size_t) _PyThread_preferred_stacksize(void);

#ifdef __cplusplus
}
#endif
#endif /* !Py_INTERNAL_THREAD_H */
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix stack overflow in signal handlers managed by :mod:`faulthandler` -
add new :c:func:`_PyThread_preferred_stack_size` and use this to find an
appropriate stack size in preference to SIGSTKSZ.
3 changes: 2 additions & 1 deletion Modules/faulthandler.c
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "Python.h"
#include "pycore_initconfig.h"
#include "pycore_traceback.h"
#include "pycore_thread.h"
#include "pythread.h"
#include <signal.h>
#include <object.h>
Expand Down Expand Up @@ -1325,7 +1326,7 @@ _PyFaulthandler_Init(int enable)
* be able to allocate memory on the stack, even on a stack overflow. If it
* fails, ignore the error. */
stack.ss_flags = 0;
stack.ss_size = SIGSTKSZ;
stack.ss_size = _PyThread_preferred_stacksize();
stack.ss_sp = PyMem_Malloc(stack.ss_size);
if (stack.ss_sp != NULL) {
err = sigaltstack(&stack, &old_stack);
Expand Down
10 changes: 10 additions & 0 deletions Python/thread_nt.h
Original file line number Diff line number Diff line change
Expand Up @@ -496,3 +496,13 @@ PyThread_tss_get(Py_tss_t *key)
SetLastError(error);
return result;
}

size_t
_PyThread_preferred_stacksize()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
_PyThread_preferred_stacksize()
_PyThread_preferred_stacksize(void)

{
/*
* Windows default stack size is 1MB, overridable by the linker in the
* executable
*/
return 1024 * 1024;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not comfortable with hardcoded values, especially the comment "overridable by the linker in the executable". Maybe we should get the size written by the linker in this case? We don't really need this function for faulthandler, maybe remove the Windows implementation?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure - I added it for completeness/documentation, but it's not called. I can remove it.

}
105 changes: 71 additions & 34 deletions Python/thread_pthread.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,41 +24,58 @@
# include <lwp.h> /* _lwp_self() */
#endif

/* The POSIX spec requires that use of pthread_attr_setstacksize
be conditional on _POSIX_THREAD_ATTR_STACKSIZE being defined. */
#ifdef _POSIX_THREAD_ATTR_STACKSIZE
#ifndef THREAD_STACK_SIZE
#define THREAD_STACK_SIZE 0 /* use default stack size */
#endif

/* The default stack size for new threads on OSX and BSD is small enough that
* we'll get hard crashes instead of 'maximum recursion depth exceeded'
* exceptions.
*
* The default stack sizes below are the empirically determined minimal stack
* sizes where a simple recursive function doesn't cause a hard crash.
*/
#if defined(__APPLE__) && defined(THREAD_STACK_SIZE) && THREAD_STACK_SIZE == 0
#undef THREAD_STACK_SIZE
#if defined(__APPLE__)
/* Note: This matches the value of -Wl,-stack_size in configure.ac */
#define THREAD_STACK_SIZE 0x1000000

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously, THREAD_STACK_SIZE was redefined if THREAD_STACK_SIZE=0. With your patch, it seems like it's always overriden: THREAD_STACK_SIZE value is no longer tested. I would prefer to minimize changes in this PR.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The end result is the same, but with less code - Originally THREAD_STACK_SIZE was always defined to 0 if not previously defined, and then was undefined/redefined if it was 0 in response to the platform detection macros. The code here was really just trying to extract the platform's preferred stack size (as reported by pthreads), and to not replicate this between the threads and faulthandler modules. Let me minimize the diff then.

#endif
#if defined(__FreeBSD__) && defined(THREAD_STACK_SIZE) && THREAD_STACK_SIZE == 0
#undef THREAD_STACK_SIZE
#define THREAD_STACK_SIZE 0x400000
#define PLATFORM_STACK_SIZE 0x1000000
#elif defined(__FreeBSD__)
#define PLATFORM_STACK_SIZE 0x400000
#elif defined(_AIX)
#define PLATFORM_STACK_SIZE 0x200000
#else
#define PLATFORM_STACK_SIZE 0 /* use system default */
#endif
#if defined(_AIX) && defined(THREAD_STACK_SIZE) && THREAD_STACK_SIZE == 0
#undef THREAD_STACK_SIZE
#define THREAD_STACK_SIZE 0x200000

/* The POSIX spec requires that use of pthread_attr_setstacksize
be conditional on _POSIX_THREAD_ATTR_STACKSIZE being defined. */
#ifdef _POSIX_THREAD_ATTR_STACKSIZE
#ifndef THREAD_STACK_SIZE
#define THREAD_STACK_SIZE PLATFORM_STACK_SIZE
#endif
/* for safety, ensure a viable minimum stacksize */
#define THREAD_STACK_MIN 0x8000 /* 32 KiB */
#else /* !_POSIX_THREAD_ATTR_STACKSIZE */
#ifdef THREAD_STACK_SIZE
#error "THREAD_STACK_SIZE defined but _POSIX_THREAD_ATTR_STACKSIZE undefined"
#endif
#endif

/* for safety, ensure a viable minimum stacksize. Either the specified pthread
* minimum, or a floor of 32KiB
*
* XXX: note this is lower than the emprical minimum for
* __APPLE__/__FreeBSD__ above
*/
#if defined(PTHREAD_STACK_MIN) && PTHREAD_STACK_MIN > 0x8000 /* 32KiB */
#define THREAD_STACK_MIN PTHREAD_STACK_MIN
#else
#define THREAD_STACK_MIN 0x8000 /* 32 KiB */
#endif

/*
* Static preferred size for a stack on the platform. Use if we can't make a
* good guess dynamically. Maximum of PLATFORM_STACK_SIZE and THREAD_STACK_MIN.
*/
#if THREAD_STACK_MIN > PLATFORM_STACK_SIZE
#define PLATFORM_STACK_PREFERRED THREAD_STACK_MIN
#else
#define PLATFORM_STACK_PREFERRED PLATFORM_STACK_SIZE
#endif

/* The POSIX spec says that implementations supporting the sem_*
family of functions must indicate this by defining
_POSIX_SEMAPHORES. */
Expand Down Expand Up @@ -241,9 +258,6 @@ PyThread_start_new_thread(void (*func)(void *), void *arg)
#if defined(THREAD_STACK_SIZE) || defined(PTHREAD_SYSTEM_SCHED_SUPPORTED)
pthread_attr_t attrs;
#endif
#if defined(THREAD_STACK_SIZE)
size_t tss;
#endif

dprintf(("PyThread_start_new_thread called\n"));
if (!initialized)
Expand All @@ -256,9 +270,11 @@ PyThread_start_new_thread(void (*func)(void *), void *arg)
#if defined(THREAD_STACK_SIZE)
PyThreadState *tstate = _PyThreadState_GET();
size_t stacksize = tstate ? tstate->interp->pythread_stacksize : 0;
tss = (stacksize != 0) ? stacksize : THREAD_STACK_SIZE;
if (tss != 0) {
if (pthread_attr_setstacksize(&attrs, tss) != 0) {
if (stacksize == 0) {
stacksize = THREAD_STACK_SIZE;
}
if (stacksize != 0) {
if (pthread_attr_setstacksize(&attrs, stacksize) != 0) {
pthread_attr_destroy(&attrs);
return PYTHREAD_INVALID_THREAD_ID;
}
Expand Down Expand Up @@ -706,7 +722,6 @@ _pythread_pthread_set_stacksize(size_t size)
{
#if defined(THREAD_STACK_SIZE)
pthread_attr_t attrs;
size_t tss_min;
int rc = 0;
#endif

Expand All @@ -717,17 +732,11 @@ _pythread_pthread_set_stacksize(size_t size)
}

#if defined(THREAD_STACK_SIZE)
#if defined(PTHREAD_STACK_MIN)
tss_min = PTHREAD_STACK_MIN > THREAD_STACK_MIN ? PTHREAD_STACK_MIN
: THREAD_STACK_MIN;
#else
tss_min = THREAD_STACK_MIN;
#endif
if (size >= tss_min) {
if (size >= THREAD_STACK_MIN) {
/* validate stack size by setting thread attribute */
if (pthread_attr_init(&attrs) == 0) {
rc = pthread_attr_setstacksize(&attrs, size);
pthread_attr_destroy(&attrs);
(void)pthread_attr_destroy(&attrs);
if (rc == 0) {
_PyInterpreterState_GET_UNSAFE()->pythread_stacksize = size;
return 0;
Expand Down Expand Up @@ -872,3 +881,31 @@ PyThread_tss_get(Py_tss_t *key)
assert(key != NULL);
return pthread_getspecific(key->_key);
}

/*
* Provide the preferred stack size to use if we need to create a stack and
* need to know the actual amount of space to allocate.
*/
size_t
_PyThread_preferred_stacksize(void)
{
#if defined(THREAD_STACK_SIZE) && (THREAD_STACK_SIZE == 0)
/* If we have the stacksize thread attribute, we can use it to find the
* system's preferred stack size, and use that.
*/
pthread_attr_t attrs;
if (pthread_attr_init(&attrs) == 0) {
size_t size;
int rc = pthread_attr_getstacksize(&attrs, &size);
(void)pthread_attr_destroy(&attrs);
/* Ensure the default size is at least PLATFORM_STACK_PREFERRED for
* systems that default to a stack too small for our needs. */
if (rc == 0 && size != 0 && size > PLATFORM_STACK_PREFERRED) {
return size;
}
}
#endif
/* If all else fails, use the minimum viable thread stack size for the
* platform. */
return PLATFORM_STACK_PREFERRED;
}