Skip to content

hts_newthread() leaves the pthread attributes object undestroyed when thread creation fails #772

Description

@xroche

hts_newthread() (src/htsthread.c) initialises a pthread_attr_t, sets a stack size on it, creates the thread, and destroys the attributes only on the success path:

    if (pthread_attr_init(&attr) != 0
        || pthread_attr_setstacksize(&attr, stackSize) != 0
        || (retcode =
            pthread_create(&handle, &attr, hts_entry_point, s_args)) != 0) {
      process_chain_add(-1);
      freet(s_args);
      return -1;
    } else {
      pthread_detach(handle);
      pthread_attr_destroy(&attr);
    }

If pthread_attr_init() succeeded and either pthread_attr_setstacksize() or pthread_create() then failed, attr is left initialised and never destroyed. pthread_create() failing is the realistic case, under EAGAIN from thread or memory exhaustion.

Low severity, and not a one-line fix. POSIX lets an implementation allocate for an attributes object, but glibc's does not for a default-initialised one, so on Linux the destroy is close to a no-op and nothing actually leaks. It is a portability and hygiene problem rather than a live bug.

The reason it needs restructuring rather than an added pthread_attr_destroy(): the three calls share one short-circuit condition, so the failure branch cannot tell which one failed, and destroying an attributes object that pthread_attr_init() never initialised is itself undefined. The init has to be split out of the chain first.

Found while reviewing #752.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions