From 5a455acd3bdf9a0117f909450ac7ea693ca93912 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Wed, 17 Oct 2018 23:21:17 +0300 Subject: [PATCH] Fix hangs due to watermarks overruns in bufferevents implementations Some implementations of bufferevents (for example openssl) can overrun read high watermark. And after this if user callback will not drain enough data it will be suspended (i.e. it will not be runned again anymore). This is not the expecting behaviour as one may guess, since in this case the data will never be read. Hence once we detected that the watermark exceeded (even after calling user callback) we will schedule the callback again. This also can be fixed in bufferevent openssl implementation (by strictly limiting how much data is added to the read buffer according to read high watermark), but since this data is already available (and in memory) there is no point in doing so. --- bufferevent.c | 25 +++++++++++++++++++++++++ include/event2/bufferevent.h | 3 +++ 2 files changed, 28 insertions(+) diff --git a/bufferevent.c b/bufferevent.c index 490b5983..89ad6e2b 100644 --- a/bufferevent.c +++ b/bufferevent.c @@ -111,6 +111,28 @@ bufferevent_unsuspend_write_(struct bufferevent *bufev, bufferevent_suspend_flag BEV_UNLOCK(bufev); } +/** + * Sometimes bufferevent's implementation can overrun high watermarks + * (one of examples is openssl) and in this case if the read callback + * will not handle enough data do over condition above the read + * callback will never be called again (due to suspend above). + * + * To avoid this we are scheduling read callback again here, but only + * from the user callback to avoid multiple scheduling: + * - when the data had been added to it + * - when the data had been drained from it (user specified read callback) + */ +static void bufferevent_inbuf_wm_check(struct bufferevent *bev) +{ + if (!bev->wm_read.high) + return; + if (!(bev->enabled & EV_READ)) + return; + if (evbuffer_get_length(bev->input) < bev->wm_read.high) + return; + + bufferevent_trigger(bev, EV_READ, BEV_OPT_DEFER_CALLBACKS); +} /* Callback to implement watermarks on the input buffer. Only enabled * if the watermark is set. */ @@ -147,6 +169,7 @@ bufferevent_run_deferred_callbacks_locked(struct event_callback *cb, void *arg) if (bufev_private->readcb_pending && bufev->readcb) { bufev_private->readcb_pending = 0; bufev->readcb(bufev, bufev->cbarg); + bufferevent_inbuf_wm_check(bufev); } if (bufev_private->writecb_pending && bufev->writecb) { bufev_private->writecb_pending = 0; @@ -187,6 +210,7 @@ bufferevent_run_deferred_callbacks_unlocked(struct event_callback *cb, void *arg void *cbarg = bufev->cbarg; bufev_private->readcb_pending = 0; UNLOCKED(readcb(bufev, cbarg)); + bufferevent_inbuf_wm_check(bufev); } if (bufev_private->writecb_pending && bufev->writecb) { bufferevent_data_cb writecb = bufev->writecb; @@ -230,6 +254,7 @@ bufferevent_run_readcb_(struct bufferevent *bufev, int options) SCHEDULE_DEFERRED(p); } else { bufev->readcb(bufev, bufev->cbarg); + bufferevent_inbuf_wm_check(bufev); } } diff --git a/include/event2/bufferevent.h b/include/event2/bufferevent.h index 825918e3..dac34dc7 100644 --- a/include/event2/bufferevent.h +++ b/include/event2/bufferevent.h @@ -518,6 +518,9 @@ int bufferevent_set_timeouts(struct bufferevent *bufev, On input, a bufferevent does not invoke the user read callback unless there is at least low watermark data in the buffer. If the read buffer is beyond the high watermark, the bufferevent stops reading from the network. + But be aware that bufferevent input/read buffer can overrun high watermark + limit (typical example is openssl bufferevent), so you should not relay in + this. On output, the user write callback is invoked whenever the buffered data falls below the low watermark. Filters that write to this bufev will try -- 2.40.0