Hi,

due to that other thread, it occurred to me that getaddrinfo() also has
another bug: It leaks memory. _asr_use_resolver() allocates memory
per-thread (via _asr_resolver()) and saves it via _THREAD_PRIVATE() in
_asr, but nothing frees that memory. A reproducer follows bellow. On
Debian, no memory leak is observed (= RES in top stays constant over time).

I have no good suggestion for how to fix this leak, but I feel like this
might also be helpful in fixing the thread unsafety from "that other
thread". Both bugs originate from storing a pointer to an allocation via
_THREAD_PRIVATE(), which is something that does not really work with
that API.

IMHO this internal API needs to change. At this point, one can also fix
the other problem by having an API that guarantees that each thread gets
zeroed per-thread data instead of memcpy()ing from a global default.

Other users of _THREAD_PRIVATE() instead seem to only store buffers,
e.g. strerror_l() or localtime(). These buffers do not need extra cleanup.

Reproducer (sorry for the line wrapping; this is basically just the
previous example, but without calling getaddrinfo() on the main thread:
lots of threads are started and each thread calls getaddrinfo() once):

#include <pthread.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netdb.h>
#include <stdio.h>
#include <string.h>

#define NUM_THREADS 50

static void do_lookup(const char *host)
{
        int s;
        struct addrinfo hints;
        struct addrinfo *result;

        memset(&hints, 0, sizeof(hints));
        hints.ai_family = AF_UNSPEC;
        hints.ai_socktype = SOCK_STREAM;
        hints.ai_flags = AI_ADDRCONFIG;
        hints.ai_protocol = IPPROTO_TCP;

        s = getaddrinfo(host, NULL, &hints, &result);
        if (s != 0) {
                fprintf(stderr, "Lookup error for %s: %s\n", host, 
gai_strerror(s));
        } else {
                freeaddrinfo(result);
        }
}

static void *
do_things(void *arg)
{
        (void) arg;
        do_lookup("ipv4.google.com");
        return NULL;
}

int main()
{
        pthread_t threads[NUM_THREADS];
        int i;
        int s;

        for (;;) {
                for (i = 0; i < NUM_THREADS; i++) {
                        s = pthread_create(&threads[i], NULL, do_things, NULL);
                        if (s != 0)
                                fprintf(stderr, "Error creating thread");
                }
                for (i = 0; i < NUM_THREADS; i++) {
                        pthread_join(threads[i], NULL);
                }
        }
        return 0;
}

Cheers,
Uli
-- 
This can be a, a little complicated. Listen, my advice is... ask
somebody else for advice, at least someone who's... got more experience
at...  giving advice.

Reply via email to