Setting a bufferevent timeout to NULL does not work about libevent

[diwakergupta]

Here's a sample program:

#include <errno.h>
#include <stdio.h>
#include <time.h>

#include <event.h>

void read_callback(struct bufferevent* be, void* ctx) {
    printf("Got read callback, disabling timeout\n");
    // This *should* disable READ and WRITE timeouts, but doesn't
    bufferevent_set_timeouts(be, NULL, NULL);
}

void write_callback(struct bufferevent* be, void* ctx) {
    printf("Finished write\n");
}

void event_callback(struct bufferevent* be, int16_t what, void* ctx) {
    printf("Got a callback %d\n", what);
    if (what & BEV_EVENT_CONNECTED) {
        printf("CONNECTED\n");
        bufferevent_enable(be, EV_READ | EV_WRITE);
        bufferevent_setcb(be, read_callback, write_callback, event_callback, ctx);
        // set a read timeout of 2 seconds
        struct timeval tv = {2, 0};

        bufferevent_set_timeouts(be, &tv, NULL);
        char* hello = "hello";
        bufferevent_write(be, (uint8_t*) hello, 5);
        return;
    }

    if (what & BEV_EVENT_EOF) {
        printf("EOF\n");
    }
    if (what & BEV_EVENT_TIMEOUT) {
        printf("TIMEOUT\n");
    }
    if (what & BEV_EVENT_ERROR) {
        printf("ERROR %d\n", evutil_socket_error_to_string(EVUTIL_SOCKET_ERROR()));
    }
    if (what & BEV_EVENT_READING) {
        printf("READING\n");
    }
    if (what & BEV_EVENT_WRITING) {
        printf("WRITING\n");
    }
    struct event_base* eb = (struct event_base*) ctx;
    event_base_loopbreak(eb);
}

int main(int argc, char* argv[]) {
    evthread_use_pthreads();

    struct event_base* eb = event_base_new();
    struct bufferevent* be = bufferevent_socket_new(eb, -1,
        BEV_OPT_CLOSE_ON_FREE | BEV_OPT_DEFER_CALLBACKS |
        BEV_OPT_THREADSAFE | BEV_OPT_UNLOCK_CALLBACKS);
    bufferevent_setcb(be, NULL, NULL, event_callback, eb);

    if (bufferevent_socket_connect_hostname(be, NULL, AF_INET,
            "localhost", 21234) < 0) {
        printf("Failed");
    }

    printf("Starting event loop\n");
    event_base_dispatch(eb);
    printf("Exiting event loop\n");
    bufferevent_free(be);
    event_base_free(eb);
}

I run it against a simple echo server. What I expect to see is that we get a connection callback, then a write callback followed by a read callback. After that, the program should just loop waiting for more I/O but instead, I still see the 2s timeout even though the read_callback explicitly disables all timeouts on that bufferevent.

[diwakergupta]

I believe the problem is the way adj_timeouts is invoked. Since I'm using bufferevent_socket_new, adj_timeouts maps to be_socket_adj_timeouts. However, that function does not do all of the cleanup done by _bufferevent_generic_adj_timeouts. In particular, even though the caller passes in NULL values, the ev_read/ev_write events never get deleted.

[diwakergupta]

FWIW the adj_timeout implementation in bufferevent_openssl suffers from the same bug.

[nmathewson]

Ugh; the real problem here is that the code assumes that "event_add(ev, NULL);" will clear an existing timeout on an event, but it won't. I'm leery of changing the behavior of event_add(ev,NULL), though, since existing code may be relying on it. I think we need a new API for removing only the timeout on an event but leaving the rest added.

Calling event_del() isn't right here, since we don't want to remove the read event--we only want to remove its timeout. Nor can we say event_del(ev); event_add(ev,NULL); -- that would lose an invocation of the event if it's currently activated.

[nmathewson]

Oh say; the underlying issue is #5.

[nmathewson]

Should work now in master -- I added the new API I mentioned above.