Fix bufferevent_pair to properly set BEV_EVENT_{READING,WRITING} on flush.

Here's some fun. From `bufferevent.h`:
  ```
  #define BEV_EVENT_READING	0x01	/**< error encountered while reading */
  #define BEV_EVENT_WRITING	0x02	/**< error encountered while writing */
  ```

And from `event.h`:
  ```
  /** Wait for a socket or FD to become readable */
  #define EV_READ		0x02
  /** Wait for a socket or FD to become writeable */
  #define EV_WRITE	0x04
  ```

Library users have to be very careful to get this right; it turns out, the
library itself got this wrong in the `bufferevent_pair` code. It appears that
in most of the code, only `BEV_EVENT_FINISHED` will indicate whether it's read
or write; on error or timeout, it appears that "both" is assumed and not set in
the callback. I read through all the other places where `BEV_EVENT_FINISHED` is
passed to an event callback; it appears that the pair code is the only spot
that got it wrong.

azat: add TT_FORK to avoid breaking clean env, and rebase commit message
(copied from #359)
Fixes: #359

diff --git a/bufferevent_pair.c b/bufferevent_pair.c
index d80e5f81d6973a3297ea4bf646c6624d7cf80dfb..1e93f57293d2e0b4b90c6b8216d428911407334d 100644 (file)
--- a/bufferevent_pair.c
+++ b/bufferevent_pair.c
@@ -325,7 +325,12 @@ be_pair_flush(struct bufferevent *bev, short iotype,
                be_pair_transfer(bev, partner, 1);
 
        if (mode == BEV_FINISHED) {
-               bufferevent_run_eventcb_(partner, iotype|BEV_EVENT_EOF, 0);
+               short what = BEV_EVENT_EOF;
+               if (iotype & EV_READ)
+                       what |= BEV_EVENT_WRITING;
+               if (iotype & EV_WRITE)
+                       what |= BEV_EVENT_READING;
+               bufferevent_run_eventcb_(partner, what, 0);
        }
        decref_and_unlock(bev);
        return 0;

diff --git a/test/regress_bufferevent.c b/test/regress_bufferevent.c
index 33ddbda34bfaeaeb959e25fa377b2beb5c024eb9..6f214a7dafc16707e266a799e1f79f3e4489f556 100644 (file)
--- a/test/regress_bufferevent.c
+++ b/test/regress_bufferevent.c
@@ -1190,6 +1190,42 @@ end:
                bufferevent_free(bev);
 }
 
+static void
+pair_flush_eventcb(struct bufferevent *bev, short what, void *ctx)
+{
+       int *callback_what = ctx;
+       *callback_what = what;
+}
+
+static void
+test_bufferevent_pair_flush(void *arg)
+{
+       struct basic_test_data *data = arg;
+       struct bufferevent *pair[2];
+       struct bufferevent *bev1 = NULL;
+       struct bufferevent *bev2 = NULL;
+       int callback_what = 0;
+
+       tt_assert(0 == bufferevent_pair_new(data->base, 0, pair));
+       bev1 = pair[0];
+       bev2 = pair[1];
+       tt_assert(0 == bufferevent_enable(bev1, EV_WRITE));
+       tt_assert(0 == bufferevent_enable(bev2, EV_READ));
+
+       bufferevent_setcb(bev2, NULL, NULL, pair_flush_eventcb, &callback_what);
+
+       bufferevent_flush(bev1, EV_WRITE, BEV_FINISHED);
+
+       event_base_loop(data->base, EVLOOP_ONCE);
+
+       tt_assert(callback_what == (BEV_EVENT_READING | BEV_EVENT_EOF));
+
+end:
+       if (bev1)
+               bufferevent_free(bev1);
+       if (bev2)
+               bufferevent_free(bev2);
+}
 
 struct testcase_t bufferevent_testcases[] = {
 
@@ -1260,6 +1296,9 @@ struct testcase_t bufferevent_testcases[] = {
        { "bufferevent_socket_filter_inactive",
          test_bufferevent_socket_filter_inactive,
          TT_FORK|TT_NEED_BASE, &basic_setup, NULL },
+       { "bufferevent_pair_flush",
+         test_bufferevent_pair_flush,
+         TT_FORK|TT_NEED_BASE, &basic_setup, NULL },
 
        END_OF_TESTCASES,
 };