[Libevent-users] bufferevent_openssl's output callback

[Catalin Patulea <catalinp@xxxxxxxxxx>]

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 majordomo@xxxxxxxxxxxxx with
unsubscribe libevent-users    in the body.