[Libevent-users] [PATCH] Fix possible memory leaks with debugging turned on.

[Leonid Evdokimov]

I think, I've found couple of memory leaks in libevent debugging code:
the event was not properly unregistered from debug hashtable.

I run evdns-example using $(valgrind --leak-check=full
--show-reachable=yes), I'm attaching three valgrind logs:

evdns-example.no-0004.no-0005.log - without 0004-*.patch
evdns-example.yes-0004.no-0005.log - only 0004-*.patch was applied
evdns-example.yes-0004.yes-0005.log - both 0004-*.patch and
0005-*.patch were applied

Probably, the leaks are not real leaks but my misunderstanding of
debugging code, so review the patches, please :)

Also, feel free to include evdns-example.c as sample of proper evdns
cleanup if you find it useful. I think, that cleanup is not trivial.

--
WBRBW, Leonid Evdokimov
xmpp:l...@darkk.net.ru && http://darkk.net.ru
tel:+79816800702 && tel:+79050965222

From 573b0ee284e19d2f399e67d58179252e2f8f7166 Mon Sep 17 00:00:00 2001
From: Leonid Evdokimov <da...@yandex-team.ru>
Date: Thu, 11 Aug 2011 03:10:08 +0400
Subject: [PATCH 4/5] Fix evsig_dealloc memory leak with debugging turned on.

---
 signal.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/signal.c b/signal.c
index 72c0847..2079204 100644
--- a/signal.c
+++ b/signal.c
@@ -400,9 +400,11 @@ evsig_dealloc(struct event_base *base)
 	int i = 0;
 	if (base->sig.ev_signal_added) {
 		event_del(&base->sig.ev_signal);
-		event_debug_unassign(&base->sig.ev_signal);
 		base->sig.ev_signal_added = 0;
 	}
+	/* debug event is created in evsig_init/event_assign even when
+	 * ev_signal_added == 0, so unassign is required */
+	event_debug_unassign(&base->sig.ev_signal);
 
 	for (i = 0; i < NSIG; ++i) {
 		if (i < base->sig.sh_old_max && base->sig.sh_old[i] != NULL)
-- 
1.7.4.1


From d95b04da4174f3843321518c3b220671979e5827 Mon Sep 17 00:00:00 2001
From: Leonid Evdokimov <da...@yandex-team.ru>
Date: Thu, 11 Aug 2011 03:24:06 +0400
Subject: [PATCH 5/5] Fix request_finished memory leak with debugging turned on.

---
 evdns.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/evdns.c b/evdns.c
index b6c2916..d7a3093 100644
--- a/evdns.c
+++ b/evdns.c
@@ -647,6 +647,8 @@ request_finished(struct request *const req, struct request **head, int free_hand
 	} else {
 		base->global_requests_waiting--;
 	}
+	/* it was initialized during request_new / evtimer_assign */
+	event_debug_unassign(&req->timeout_event);
 
 	if (!req->request_appended) {
 		/* need to free the request data on it's own */
-- 
1.7.4.1


#include <event2/dns.h>
#include <event2/event.h>
#include <assert.h>
#include <stdlib.h>
#include <stdio.h>

void evdns_callback(int result, char type, int count, int ttl, void *addresses, void *arg)
{
    (void)type;
    (void)count;
    (void)ttl;
    (void)addresses;
    (void)arg;
    puts(evdns_err_to_string(result));
}

void test_cancel()
{
    struct event_base* evbase = event_base_new();
    assert(evbase);
    struct evdns_base *evdns = evdns_base_new(evbase, 1);
    assert(evdns);
    struct evdns_request* req = evdns_base_resolve_ipv4(
            evdns, "www.google.com", DNS_QUERY_NO_SEARCH, evdns_callback, 0);
    evdns_cancel_request(evdns, req);
    req = 0;

    // `req` is freed in callback, that's why one loop is required.
    event_base_loop(evbase, EVLOOP_NONBLOCK);

    int send_err_shutdown = 1; // means nothing as soon as our request is already canceled
    evdns_base_free(evdns, send_err_shutdown);
    evdns = 0;
    event_base_free(evbase);
    evbase = 0;
}

void test_shutdown(int send_err_shutdown)
{
    struct event_base* evbase = event_base_new();
    assert(evbase);
    struct evdns_base *evdns = evdns_base_new(evbase, 1);
    assert(evdns);
    struct evdns_request* req = evdns_base_resolve_ipv4(
            evdns, "www.google.com", DNS_QUERY_NO_SEARCH, evdns_callback, 0);
    req = 0;

    // `req` is freed both with `send_err_shutdown` and without it, the only
    // difference is `evdns_callback` call
    evdns_base_free(evdns, send_err_shutdown);
    evdns = 0;

    // `req` is freed in callback, that's why one loop is required.
    event_base_loop(evbase, EVLOOP_NONBLOCK);

    event_base_free(evbase);
    evbase = 0;
}

int main(int argc, char* argv[])
{
    if (argc != 3) {
        puts("Usage: evdns-example <use_debug> <testcase>");
        return 1;
    }

    if (atoi(argv[1])) {
        // may leak a bit, at least it leaves traces in valgrind --leak-check=full --show-reachable=yes
        event_enable_debug_mode();
    }

    switch (atoi(argv[2])) {
    case 1:
        test_cancel();
        break;
    case 2:
        test_shutdown(0);
        break;
    case 3:
        test_shutdown(1);
        break;
    default:
        puts("Unknown testcase");
    }
    return 0;
}