Discussion:
bufferevent_openssl's output callback
(too old to reply)
Catalin Patulea
2014-10-06 17:17:43 UTC
Permalink
Raw Message
I'm trying to get evhttp+bufferevent_openssl working without
BEV_OPT_DEFER_CALLBACKS because I suspect it's the cause of some
crashes on connection close edge cases.

Just dropping DEFER_CALLBACKS seem to cause races between setting the
callback and adding into the buffer. In some cases, add_buffer wants
to trigger EV_WRITE, but the cb isn't set yet, and the cb never ends
up getting called. This is similar to
https://github.com/libevent/libevent/commit/5eb178855a7263a50e38139089720fef7c3a1642
but I think the issue is more widespread.

bufferevent_openssl does SSL_write as soon as the client writes to outbuf:

static void
be_openssl_outbuf_cb(struct evbuffer *buf,
const struct evbuffer_cb_info *cbinfo, void *arg)
{
[...]
if (cbinfo->n_added && bev_ssl->state == BUFFEREVENT_SSL_OPEN) {
if (cbinfo->orig_size == 0)
r = bufferevent_add_event_(&bev_ssl->bev.bev.ev_write,
&bev_ssl->bev.bev.timeout_write);
consider_writing(bev_ssl);
}
[...]
}

consider_writing() in fact writes, the write completes and does
bufferevent_trigger(EV_WRITE), but http.c hasn't set its cb yet.

By contrast, bufferevent_sock just adds an EV_WRITE and flushes only
in the callback. By that time, evhttp has set its cb and returned back
to the main loop, so the cb is called.

static void
bufferevent_socket_outbuf_cb(struct evbuffer *buf,
const struct evbuffer_cb_info *cbinfo,
void *arg)
{
[...]
if (cbinfo->n_added &&
(bufev->enabled & EV_WRITE) &&
!event_pending(&bufev->ev_write, EV_WRITE, NULL) &&
!bufev_p->write_suspended) {
if (be_socket_add(&bufev->ev_write, &bufev->timeout_write) == -1) {
[...]
}
}
}

I've dropped consider_writing from bufferevent_openssl and this has
allowed evhttp to work correctly without DEFER_CALLBACKS.

I guess it comes down to: does evbuffer_add, or less generally,
bufferevent output buffers, make any guarantees about when the
callback will be called? (eg. never inline?) Seems evhttp implicitly
makes this assumption because bufferevent_sock's always worked this
way.

Catalin
***********************************************************************
To unsubscribe, send an e-mail to ***@freehaven.net with
unsubscribe libevent-users in the body.
Catalin Patulea
2014-10-07 05:25:45 UTC
Permalink
Raw Message
Here's what the crash I'm hunting looks like:

$ sample/https-client -url https://ssl.tabimado.net/sight/map-guangdong-fs.htm
# this particular URL seems to trigger it about 1:15 times
[...]
[err] http.c:691: Assertion req != NULL failed in evhttp_connection_fail_
* thread #1: tid = 0xd3664, 0x00007fff87375866
libsystem_kernel.dylib`__pthread_kill + 10, queue =
'com.apple.main-thread', stop reason = signal SIGABRT
* frame #0: 0x00007fff87375866 libsystem_kernel.dylib`__pthread_kill + 10
frame #1: 0x00007fff8cb4835c libsystem_pthread.dylib`pthread_kill + 92
frame #2: 0x00007fff8a245b1a libsystem_c.dylib`abort + 125
frame #3: 0x000000010002c0c2 https-client`event_exit + 66
frame #4: 0x000000010002c72c https-client`event_errx + 380
frame #5: 0x00000001000307db https-client`evhttp_connection_fail_ + 123
frame #6: 0x0000000100032fb8 https-client`evhttp_error_cb + 712
frame #7: 0x000000010000ce91
https-client`bufferevent_run_deferred_callbacks_locked + 561
frame #8: 0x0000000100020ca6
https-client`event_process_active_single_queue + 1254
frame #9: 0x00000001000193e3 https-client`event_process_active + 339
frame #10: 0x0000000100018173 https-client`event_base_loop + 883

Full debug_mode log: http://sprunge.us/RQDa

I'm guessing it's some race between evhttp tearing down the
bufferevent and getting an EOF event from the server.
Post by Catalin Patulea
I'm trying to get evhttp+bufferevent_openssl working without
BEV_OPT_DEFER_CALLBACKS because I suspect it's the cause of some
crashes on connection close edge cases.
Just dropping DEFER_CALLBACKS seem to cause races between setting the
callback and adding into the buffer. In some cases, add_buffer wants
to trigger EV_WRITE, but the cb isn't set yet, and the cb never ends
up getting called. This is similar to
https://github.com/libevent/libevent/commit/5eb178855a7263a50e38139089720fef7c3a1642
but I think the issue is more widespread.
static void
be_openssl_outbuf_cb(struct evbuffer *buf,
const struct evbuffer_cb_info *cbinfo, void *arg)
{
[...]
if (cbinfo->n_added && bev_ssl->state == BUFFEREVENT_SSL_OPEN) {
if (cbinfo->orig_size == 0)
r = bufferevent_add_event_(&bev_ssl->bev.bev.ev_write,
&bev_ssl->bev.bev.timeout_write);
consider_writing(bev_ssl);
}
[...]
}
consider_writing() in fact writes, the write completes and does
bufferevent_trigger(EV_WRITE), but http.c hasn't set its cb yet.
By contrast, bufferevent_sock just adds an EV_WRITE and flushes only
in the callback. By that time, evhttp has set its cb and returned back
to the main loop, so the cb is called.
static void
bufferevent_socket_outbuf_cb(struct evbuffer *buf,
const struct evbuffer_cb_info *cbinfo,
void *arg)
{
[...]
if (cbinfo->n_added &&
(bufev->enabled & EV_WRITE) &&
!event_pending(&bufev->ev_write, EV_WRITE, NULL) &&
!bufev_p->write_suspended) {
if (be_socket_add(&bufev->ev_write, &bufev->timeout_write) == -1) {
[...]
}
}
}
I've dropped consider_writing from bufferevent_openssl and this has
allowed evhttp to work correctly without DEFER_CALLBACKS.
I guess it comes down to: does evbuffer_add, or less generally,
bufferevent output buffers, make any guarantees about when the
callback will be called? (eg. never inline?) Seems evhttp implicitly
makes this assumption because bufferevent_sock's always worked this
way.
Catalin
***********************************************************************
To unsubscribe, send an e-mail to ***@freehaven.net with
unsubscribe libevent-users in the body.

Loading...