Commit 9beb25cb authored by jan.koester's avatar jan.koester
Browse files

threads: failed creation is EAGAIN/system_error; 4 MiB stacks on 32 bits 

fenrirds on ppc crashed in pthread_join() right after starting: libnetplus
starts a thread per CPU and listening socket (~300 under qemu on 32 CPUs),
and 300 default stacks of 16 MiB don't fit in a 32-bit address space.

- std::thread ignored pthread_create()'s result: a failed thread stayed
  joinable() with an uninitialized handle and join() dereferenced it. It
  now throws system_error (EAGAIN); join() of a non-joinable thread throws
  instead of crashing
- pthread_create() returned -1 instead of an error number and leaked the
  thread object; now EAGAIN, the object freed. pthread_join(): ESRCH for
  no thread, EINVAL instead of -1
- default thread stack: 4 MiB on 32-bit targets (16 MiB on 64-bit as before)

New e2e_thread_create_fail; 158/158 x86_64, 156/156 ppc (qemu).

Co-Authored-By: default avatarClaude Opus 5.5 <noreply@anthropic.com>
parent 68315df7

include/cxx/thread

+18 −3
Changes for include/cxx/thread: 18 added lines, 3 removed lines.
Original line number Diff line number Diff line
@@ -37,6 +37,7 @@ 
#include <time.h>

#include <errno.h>

#include <functional>

#include <system_error>



namespace std __attribute__((__visibility__("default"))) {
@@ -72,10 +73,21 @@ public: 
    }



    template<class F, class... Args>

    explicit thread(F&& f, Args&&... args) : _started(true) {

    explicit thread(F&& f, Args&&... args) : _t(nullptr), _started(false) {

        typedef tuple<decay_t<F>, decay_t<Args>...> Bound;

        Bound* bound = new Bound(std::forward<F>(f), std::forward<Args>(args)...);

        pthread_create(&_t, nullptr, &thread::_entry<Bound>, bound);

        /* A thread that couldn't be created (no memory for its stack:

         * a few hundred threads fill a 32-bit address space) is

         * system_error, as the standard says -- it used to be ignored,

         * leaving a joinable() thread with a garbage handle whose join()

         * crashed in pthread_join(). */

        int err = pthread_create(&_t, nullptr, &thread::_entry<Bound>, bound);

        if (err != 0) {

            delete bound;

            _t = nullptr;

            throw system_error(err, generic_category(), "thread");

        }

        _started = true;

    }



    ~thread() { if (_started) __builtin_trap(); /* std::terminate: joinable() on destruction is a bug in the caller */ }
@@ -83,8 +95,11 @@ public: 
    bool joinable() const noexcept { return _started; }



    void join() {

        pthread_join(_t, nullptr);

        if (!_started) throw system_error(EINVAL, generic_category(), "thread::join");

        int err = pthread_join(_t, nullptr);

        _started = false;

        _t = nullptr;

        if (err != 0) throw system_error(err, generic_category(), "thread::join");

    }



    void detach() {

src/linux/thread.cpp

+15 −4
Changes for src/linux/thread.cpp: 15 added lines, 4 removed lines.
Original line number Diff line number Diff line
@@ -52,7 +52,10 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. 
#include "console.h"

#include "file.h"



#define DEFAULT_STACKSIZE (4096 * 4096)

/* 16 MiB on 64 bits; on 32 bits 4 MiB: 300 threads (libnetplus starts one

 * per CPU and listening socket) of 16 MiB are more than the 4 GiB address

 * space, and pthread_create() fails */

#define DEFAULT_STACKSIZE (sizeof(void*) >= 8 ? 4096 * 4096 : 4096 * 1024)



struct sys::__internal::pthread_key_struct sys::__internal::__pthread_keys[128];
@@ -859,6 +862,8 @@ extern "C" { 
    }



    int pthread_join(pthread_t thread, void** retval) {

        if (!thread)

            return ESRCH;

        try {

            /* join() must run unconditionally: it's the only thing that

             * actually waits for the real OS thread to finish (via the
@@ -878,16 +883,20 @@ extern "C" { 
            return 0;

        }

        catch (...) {

            return -1;

            return EINVAL;

        }

    }



    /* POSIX: an error number, not -1 -- std::thread passes it on as

     * system_error. EAGAIN covers what can fail here: no memory for the

     * thread's stack or TLS, clone() refused. */

    int pthread_create(pthread_t* thread,

        const pthread_attr_t* __restrict__ attr,

        void* (*start_routine)(void*),

        void* __restrict__ arg) {

        sys::posixthread* t = nullptr;

        try {

            sys::posixthread* t = attr

            t = attr

                ? new sys::posixthread(*attr, start_routine, arg)

                : new sys::posixthread(nullptr, start_routine, arg);

            /* start() must run after the posixthread object is fully
@@ -899,7 +908,9 @@ extern "C" { 
            return 0;

        }

        catch (...) {

            return -1;

            /* start() threw before clone(): no thread runs on t's stack */

            delete t;

            return EAGAIN;

        }

    }

src/test/e2e/CMakeLists.txt

+22 −0
Changes for src/test/e2e/CMakeLists.txt: 22 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -3807,3 +3807,25 @@ target_link_libraries(e2e_cpu_alloc PRIVATE 
add_dependencies(e2e_cpu_alloc crti Scrt1 crtbeginS crtn systempp)



add_test(NAME e2e_cpu_alloc COMMAND e2e_cpu_alloc)



# pthread_create()/std::thread when no thread can be created: EAGAIN and

# system_error, not a joinable thread with a garbage handle

add_executable(e2e_thread_create_fail thread_create_fail_test.cpp)

set_target_properties(e2e_thread_create_fail PROPERTIES OUTPUT_NAME "thread_create_fail_test")

target_compile_options(e2e_thread_create_fail PRIVATE -fPIE)

target_link_options(e2e_thread_create_fail PRIVATE

    -nostdlib -pie

    -Wl,--hash-style=sysv

    -Wl,--dynamic-linker,$<TARGET_FILE_DIR:systempp>/ld-linux-${CMAKE_SYSTEM_PROCESSOR}.so

    -Wl,-rpath,$<TARGET_FILE_DIR:systempp>

)

target_link_libraries(e2e_thread_create_fail PRIVATE

    $<TARGET_OBJECTS:crti>

    $<TARGET_OBJECTS:Scrt1>

    $<TARGET_OBJECTS:crtbeginS>

    $<TARGET_OBJECTS:crtn>

    systempp

)

add_dependencies(e2e_thread_create_fail crti Scrt1 crtbeginS crtn systempp)



add_test(NAME e2e_thread_create_fail COMMAND e2e_thread_create_fail)

src/test/e2e/thread_create_fail_test.cpp

0 → 100644
+69 −0
Changes for src/test/e2e/thread_create_fail_test.cpp: 69 added lines, 0 removed lines.
Original line number Diff line number Diff line 
/* A thread that can't be created: pthread_create() returns an error number

 * (not -1), pthread_join() of no thread is ESRCH, and std::thread throws

 * system_error instead of becoming joinable() with a garbage handle whose

 * join() crashed (fenrirds on 32-bit powerpc: ~300 threads of 16 MiB

 * stacks filled the address space). */

#include <pthread.h>

#include <errno.h>

#include <stdio.h>

#include <sys/resource.h>

#include <unistd.h>

#include <fcntl.h>

#include <stdlib.h>

#include <thread>

#include <system_error>



#define CHECK(c, m) do { if (!(c)) { printf("FAIL: %s\n", m); return 1; } } while (0)



static void *noop(void *) { return nullptr; }



int main() {

    /* a stack no address space has room for */

    size_t huge = sizeof(void *) >= 8 ? ((size_t)1 << 50) : (size_t)0xF0000000u;

    pthread_attr_t attr;

    CHECK(pthread_attr_init(&attr) == 0, "pthread_attr_init");

    CHECK(pthread_attr_setstacksize(&attr, huge) == 0, "pthread_attr_setstacksize");

    pthread_t t = nullptr;

    int err = pthread_create(&t, &attr, noop, nullptr);

    CHECK(err == EAGAIN, "pthread_create without room for the stack returns EAGAIN");

    pthread_attr_destroy(&attr);



    CHECK(pthread_join(nullptr, nullptr) == ESRCH, "pthread_join of no thread is ESRCH");



    /* still works afterwards */

    CHECK(pthread_create(&t, nullptr, noop, nullptr) == 0, "pthread_create");

    CHECK(pthread_join(t, nullptr) == 0, "pthread_join");



    /* std::thread: system_error, not a joinable() thread. Needs an

     * address-space limit that the default stack doesn't fit in -- qemu-user

     * doesn't apply RLIMIT_AS, so where a thread still starts, only that

     * it joins is checked. */

    struct rlimit old;

    CHECK(getrlimit(RLIMIT_AS, &old) == 0, "getrlimit");

    struct rlimit lim = old;

    /* what is mapped now (statm: pages) plus 1 MiB: no room for a stack */

    char buf[64] = {0};

    int fd = open("/proc/self/statm", O_RDONLY);

    CHECK(fd >= 0 && read(fd, buf, sizeof(buf) - 1) > 0, "read /proc/self/statm");

    close(fd);

    lim.rlim_cur = (rlim_t)strtoul(buf, nullptr, 10) * (rlim_t)sysconf(_SC_PAGESIZE) + (1u << 20);

    bool threw = false, started = false;

    if (setrlimit(RLIMIT_AS, &lim) == 0) {

        try {

            std::thread th([] {});

            started = th.joinable();

            th.join();

        } catch (const std::system_error &e) {

            threw = true;

            CHECK(e.code().value() == EAGAIN, "std::thread's system_error carries EAGAIN");

        }

        setrlimit(RLIMIT_AS, &old);

        CHECK(threw || started, "std::thread either throws or starts a joinable thread");

    }

    std::thread ok([] {});

    CHECK(ok.joinable(), "std::thread starts again");

    ok.join();

    CHECK(!ok.joinable(), "joined");

    printf("PASS%s\n", threw ? "" : " (RLIMIT_AS not applied: std::thread failure path not exercised)");

    return 0;

}