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.
hts_newthread()(src/htsthread.c) initialises apthread_attr_t, sets a stack size on it, creates the thread, and destroys the attributes only on the success path:If
pthread_attr_init()succeeded and eitherpthread_attr_setstacksize()orpthread_create()then failed,attris left initialised and never destroyed.pthread_create()failing is the realistic case, underEAGAINfrom 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 thatpthread_attr_init()never initialised is itself undefined. The init has to be split out of the chain first.Found while reviewing #752.