evthr_pool_free after evthr_pool_stop (in evhtp_free) is unable to free threads memory #69

Open
pudv opened this Issue Dec 11, 2012 · 4 comments

Projects

None yet

3 participants

@pudv
pudv commented Dec 11, 2012

void
evthr_pool_free(evthr_pool_t * pool) {
    evthr_t * thread;
    evthr_t * save;

    if (pool == NULL) {
        return;
    }

    TAILQ_FOREACH_SAFE(thread, &pool->threads, next, save) {
        TAILQ_REMOVE(&pool->threads, thread, next);

        evthr_free(thread); <--------------- unreachable after memset in evthr_pool_stop
    }

    free(pool);
}

evthr_res
evthr_pool_stop(evthr_pool_t * pool) {
    evthr_t * thr;
    evthr_t * save;

    if (pool == NULL) {
        return EVTHR_RES_FATAL;
    }

    TAILQ_FOREACH_SAFE(thr, &pool->threads, next, save) {
        evthr_stop(thr);
    }

    memset(&pool->threads, 0, sizeof(pool->threads)); <------ pool head is zero now

    return EVTHR_RES_OK;
}

@ellzey
Owner
ellzey commented Dec 11, 2012

I have no idea what I was thinking.

Thanks, will be fixed in next rel.

@fancycode

The thread is now freed in L185 inside the thread and also in "evthr_pool_free", causing a duplicate free:

==351== Invalid read of size 8
==351==    at 0x449620: evthr_free (evthr.c:341)
==351==    by 0x4496DF: _evthr_loop (evthr.c:185)
==351==    by 0x61CCE99: start_thread (pthread_create.c:308)
==351==    by 0x6CE7CBC: clone (clone.S:112)
==351==  Address 0x9b3f3d0 is 32 bytes inside a block of size 208 free'd
==351==    at 0x4C2A82E: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==351==    by 0x449862: evthr_pool_free (evthr.c:360)
==351==    ...
@pudv
pudv commented Jan 17, 2013

Double free:

static void *
_evthr_loop(void * args) {
evthr_t * thread;

if (!(thread = (evthr_t *)args)) {
    return NULL;
}

if (thread == NULL || thread->thr == NULL) {
    pthread_exit(NULL);
}

thread->evbase = event_base_new();
thread->event  = event_new(thread->evbase, thread->rdr,
                           EV_READ | EV_PERSIST, _evthr_read_cmd, args);

event_add(thread->event, NULL);

pthread_mutex_lock(&thread->lock);
if (thread->init_cb != NULL) {
    thread->init_cb(thread, thread->arg);
}
pthread_mutex_unlock(&thread->lock);

event_base_loop(thread->evbase, 0);

if (thread->err == 1) {
    fprintf(stderr, "FATAL ERROR!\n");
}

evthr_free(thread);               <-------------------- first free
pthread_exit(NULL);

}

void
evthr_pool_free(evthr_pool_t * pool) {
evthr_t * thread;
evthr_t * save;

if (pool == NULL) {
    return;
}

TAILQ_FOREACH_SAFE(thread, &pool->threads, next, save) {
    TAILQ_REMOVE(&pool->threads, thread, next);

    evthr_free(thread);          <-------------------------- second free
}

free(pool);

}

@ellzey
Owner
ellzey commented Jan 17, 2013

should probably just issue a stop instead of free here. Will get it in, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment