Skip to content

Commit

Permalink
Merge pull request zeromq#405 from gummif/gfa/active-poller-handler
Browse files Browse the repository at this point in the history
Problem: Active poller double add mutates handler
  • Loading branch information
sigiesec authored Sep 7, 2020
2 parents 7efc9b1 + d237615 commit bf4f75b
Showing 4 changed files with 111 additions and 100 deletions.
164 changes: 78 additions & 86 deletions tests/active_poller.cpp
Original file line number Diff line number Diff line change
@@ -14,9 +14,12 @@ TEST_CASE("create destroy", "[active_poller]")
}

static_assert(!std::is_copy_constructible<zmq::active_poller_t>::value,
"active_active_poller_t should not be copy-constructible");
"active_poller_t should not be copy-constructible");
static_assert(!std::is_copy_assignable<zmq::active_poller_t>::value,
"active_active_poller_t should not be copy-assignable");
"active_poller_t should not be copy-assignable");

static const zmq::active_poller_t::handler_type no_op_handler =
[](zmq::event_flags) {};

TEST_CASE("move construct empty", "[active_poller]")
{
@@ -47,9 +50,7 @@ TEST_CASE("move construct non empty", "[active_poller]")
zmq::socket_t socket{context, zmq::socket_type::router};

zmq::active_poller_t a;
a.add(socket, zmq::event_flags::pollin, [](zmq::event_flags)
{
});
a.add(socket, zmq::event_flags::pollin, [](zmq::event_flags) {});
CHECK_FALSE(a.empty());
CHECK(1u == a.size());
zmq::active_poller_t b = std::move(a);
@@ -65,9 +66,7 @@ TEST_CASE("move assign non empty", "[active_poller]")
zmq::socket_t socket{context, zmq::socket_type::router};

zmq::active_poller_t a;
a.add(socket, zmq::event_flags::pollin, [](zmq::event_flags)
{
});
a.add(socket, zmq::event_flags::pollin, no_op_handler);
CHECK_FALSE(a.empty());
CHECK(1u == a.size());
zmq::active_poller_t b;
@@ -79,12 +78,22 @@ TEST_CASE("move assign non empty", "[active_poller]")
}

TEST_CASE("add handler", "[active_poller]")
{
zmq::context_t context;
zmq::socket_t socket{context, zmq::socket_type::router};
zmq::active_poller_t active_poller;
CHECK_NOTHROW(
active_poller.add(socket, zmq::event_flags::pollin, no_op_handler));
}

TEST_CASE("add null handler fails", "[active_poller]")
{
zmq::context_t context;
zmq::socket_t socket{context, zmq::socket_type::router};
zmq::active_poller_t active_poller;
zmq::active_poller_t::handler_type handler;
CHECK_NOTHROW(active_poller.add(socket, zmq::event_flags::pollin, handler));
CHECK_THROWS_AS(active_poller.add(socket, zmq::event_flags::pollin, handler),
const std::invalid_argument &);
}

#if ZMQ_VERSION >= ZMQ_MAKE_VERSION(4, 3, 0)
@@ -94,52 +103,54 @@ TEST_CASE("add handler invalid events type", "[active_poller]")
zmq::context_t context;
zmq::socket_t socket{context, zmq::socket_type::router};
zmq::active_poller_t active_poller;
zmq::active_poller_t::handler_type handler;
short invalid_events_type = 2 << 10;
CHECK_THROWS_AS(
active_poller.add(socket, static_cast<zmq::event_flags>(invalid_events_type),
handler), const zmq::error_t&);
active_poller.add(socket, static_cast<zmq::event_flags>(invalid_events_type),
no_op_handler),
const zmq::error_t &);
CHECK(active_poller.empty());
CHECK(0u == active_poller.size());
}
#endif

TEST_CASE("add handler twice throws", "[active_poller]")
{
zmq::context_t context;
zmq::socket_t socket{context, zmq::socket_type::router};
common_server_client_setup s;

CHECK(s.client.send(zmq::message_t{}, zmq::send_flags::none));

zmq::active_poller_t active_poller;
zmq::active_poller_t::handler_type handler;
active_poller.add(socket, zmq::event_flags::pollin, handler);
/// \todo the actual error code should be checked
CHECK_THROWS_AS(active_poller.add(socket, zmq::event_flags::pollin, handler),
const zmq::error_t&);
bool message_received = false;
active_poller.add(
s.server, zmq::event_flags::pollin,
[&message_received](zmq::event_flags) { message_received = true; });
CHECK_THROWS_ZMQ_ERROR(
EINVAL, active_poller.add(s.server, zmq::event_flags::pollin, no_op_handler));
CHECK(1 == active_poller.wait(std::chrono::milliseconds{-1}));
CHECK(message_received); // handler unmodified
}

TEST_CASE("wait with no handlers throws", "[active_poller]")
{
zmq::active_poller_t active_poller;
/// \todo the actual error code should be checked
CHECK_THROWS_AS(active_poller.wait(std::chrono::milliseconds{10}),
const zmq::error_t&);
CHECK_THROWS_ZMQ_ERROR(EFAULT,
active_poller.wait(std::chrono::milliseconds{10}));
}

TEST_CASE("remove unregistered throws", "[active_poller]")
{
zmq::context_t context;
zmq::socket_t socket{context, zmq::socket_type::router};
zmq::active_poller_t active_poller;
/// \todo the actual error code should be checked
CHECK_THROWS_AS(active_poller.remove(socket), const zmq::error_t&);
CHECK_THROWS_ZMQ_ERROR(EINVAL, active_poller.remove(socket));
}

TEST_CASE("remove registered empty", "[active_poller]")
{
zmq::context_t context;
zmq::socket_t socket{context, zmq::socket_type::router};
zmq::active_poller_t active_poller;
active_poller.add(socket, zmq::event_flags::pollin,
zmq::active_poller_t::handler_type{});
active_poller.add(socket, zmq::event_flags::pollin, no_op_handler);
CHECK_NOTHROW(active_poller.remove(socket));
}

@@ -148,18 +159,15 @@ TEST_CASE("remove registered non empty", "[active_poller]")
zmq::context_t context;
zmq::socket_t socket{context, zmq::socket_type::router};
zmq::active_poller_t active_poller;
active_poller.add(socket, zmq::event_flags::pollin, [](zmq::event_flags)
{
});
active_poller.add(socket, zmq::event_flags::pollin, no_op_handler);
CHECK_NOTHROW(active_poller.remove(socket));
}

namespace
{
struct server_client_setup : common_server_client_setup
{
zmq::active_poller_t::handler_type handler = [&](zmq::event_flags e)
{
zmq::active_poller_t::handler_type handler = [&](zmq::event_flags e) {
events = e;
};

@@ -178,12 +186,11 @@ TEST_CASE("poll basic", "[active_poller]")

zmq::active_poller_t active_poller;
bool message_received = false;
zmq::active_poller_t::handler_type handler = [&message_received
](zmq::event_flags events)
{
CHECK(zmq::event_flags::none != (events & zmq::event_flags::pollin));
message_received = true;
};
zmq::active_poller_t::handler_type handler =
[&message_received](zmq::event_flags events) {
CHECK(zmq::event_flags::none != (events & zmq::event_flags::pollin));
message_received = true;
};
CHECK_NOTHROW(active_poller.add(s.server, zmq::event_flags::pollin, handler));
CHECK(1 == active_poller.wait(std::chrono::milliseconds{-1}));
CHECK(message_received);
@@ -200,8 +207,7 @@ TEST_CASE("client server", "[active_poller]")
// Setup active_poller
zmq::active_poller_t active_poller;
zmq::event_flags events;
zmq::active_poller_t::handler_type handler = [&](zmq::event_flags e)
{
zmq::active_poller_t::handler_type handler = [&](zmq::event_flags e) {
if (zmq::event_flags::none != (e & zmq::event_flags::pollin)) {
zmq::message_t zmq_msg;
CHECK_NOTHROW(s.server.recv(zmq_msg)); // get message
@@ -224,9 +230,8 @@ TEST_CASE("client server", "[active_poller]")

// Re-add server socket with pollout flag
CHECK_NOTHROW(active_poller.remove(s.server));
CHECK_NOTHROW(
active_poller.add(s.server, zmq::event_flags::pollin | zmq::event_flags::
pollout, handler));
CHECK_NOTHROW(active_poller.add(
s.server, zmq::event_flags::pollin | zmq::event_flags::pollout, handler));
CHECK(1 == active_poller.wait(std::chrono::milliseconds{-1}));
CHECK(events == zmq::event_flags::pollout);
}
@@ -237,10 +242,8 @@ TEST_CASE("add invalid socket throws", "[active_poller]")
zmq::active_poller_t active_poller;
zmq::socket_t a{context, zmq::socket_type::router};
zmq::socket_t b{std::move(a)};
CHECK_THROWS_AS(
active_poller.add(a, zmq::event_flags::pollin, zmq::active_poller_t::
handler_type{}),
const zmq::error_t&);
CHECK_THROWS_AS(active_poller.add(a, zmq::event_flags::pollin, no_op_handler),
const zmq::error_t &);
}

TEST_CASE("remove invalid socket throws", "[active_poller]")
@@ -249,12 +252,11 @@ TEST_CASE("remove invalid socket throws", "[active_poller]")
zmq::socket_t socket{context, zmq::socket_type::router};
zmq::active_poller_t active_poller;
CHECK_NOTHROW(
active_poller.add(socket, zmq::event_flags::pollin, zmq::active_poller_t::
handler_type{}));
active_poller.add(socket, zmq::event_flags::pollin, no_op_handler));
CHECK(1u == active_poller.size());
std::vector<zmq::socket_t> sockets;
sockets.emplace_back(std::move(socket));
CHECK_THROWS_AS(active_poller.remove(socket), const zmq::error_t&);
CHECK_THROWS_AS(active_poller.remove(socket), const zmq::error_t &);
CHECK(1u == active_poller.size());
}

@@ -263,8 +265,8 @@ TEST_CASE("wait on added empty handler", "[active_poller]")
server_client_setup s;
CHECK_NOTHROW(s.client.send(zmq::message_t{hi_str}, zmq::send_flags::none));
zmq::active_poller_t active_poller;
zmq::active_poller_t::handler_type handler;
CHECK_NOTHROW(active_poller.add(s.server, zmq::event_flags::pollin, handler));
CHECK_NOTHROW(
active_poller.add(s.server, zmq::event_flags::pollin, no_op_handler));
CHECK_NOTHROW(active_poller.wait(std::chrono::milliseconds{-1}));
}

@@ -274,7 +276,7 @@ TEST_CASE("modify empty throws", "[active_poller]")
zmq::socket_t socket{context, zmq::socket_type::push};
zmq::active_poller_t active_poller;
CHECK_THROWS_AS(active_poller.modify(socket, zmq::event_flags::pollin),
const zmq::error_t&);
const zmq::error_t &);
}

TEST_CASE("modify invalid socket throws", "[active_poller]")
@@ -284,7 +286,7 @@ TEST_CASE("modify invalid socket throws", "[active_poller]")
zmq::socket_t b{std::move(a)};
zmq::active_poller_t active_poller;
CHECK_THROWS_AS(active_poller.modify(a, zmq::event_flags::pollin),
const zmq::error_t&);
const zmq::error_t &);
}

TEST_CASE("modify not added throws", "[active_poller]")
@@ -293,24 +295,19 @@ TEST_CASE("modify not added throws", "[active_poller]")
zmq::socket_t a{context, zmq::socket_type::push};
zmq::socket_t b{context, zmq::socket_type::push};
zmq::active_poller_t active_poller;
CHECK_NOTHROW(
active_poller.add(a, zmq::event_flags::pollin, zmq::active_poller_t::
handler_type{}));
CHECK_NOTHROW(active_poller.add(a, zmq::event_flags::pollin, no_op_handler));
CHECK_THROWS_AS(active_poller.modify(b, zmq::event_flags::pollin),
const zmq::error_t&);
const zmq::error_t &);
}

TEST_CASE("modify simple", "[active_poller]")
{
zmq::context_t context;
zmq::socket_t a{context, zmq::socket_type::push};
zmq::active_poller_t active_poller;
CHECK_NOTHROW(active_poller.add(a, zmq::event_flags::pollin, no_op_handler));
CHECK_NOTHROW(
active_poller.add(a, zmq::event_flags::pollin, zmq::active_poller_t::
handler_type{}));
CHECK_NOTHROW(
active_poller.modify(a, zmq::event_flags::pollin | zmq::event_flags::pollout
));
active_poller.modify(a, zmq::event_flags::pollin | zmq::event_flags::pollout));
}

TEST_CASE("poll client server", "[active_poller]")
@@ -330,9 +327,8 @@ TEST_CASE("poll client server", "[active_poller]")
CHECK(s.events == zmq::event_flags::pollin);

// Modify server socket with pollout flag
CHECK_NOTHROW(
active_poller.modify(s.server, zmq::event_flags::pollin | zmq::event_flags::
pollout));
CHECK_NOTHROW(active_poller.modify(s.server, zmq::event_flags::pollin
| zmq::event_flags::pollout));
CHECK(1 == active_poller.wait(std::chrono::milliseconds{500}));
CHECK(s.events == (zmq::event_flags::pollin | zmq::event_flags::pollout));
}
@@ -346,9 +342,8 @@ TEST_CASE("wait one return", "[active_poller]")

// Setup active_poller
zmq::active_poller_t active_poller;
CHECK_NOTHROW(
active_poller.add(s.server, zmq::event_flags::pollin, [&count](zmq::
event_flags) { ++count; }));
CHECK_NOTHROW(active_poller.add(s.server, zmq::event_flags::pollin,
[&count](zmq::event_flags) { ++count; }));

// client sends message
CHECK_NOTHROW(s.client.send(zmq::message_t{hi_str}, zmq::send_flags::none));
@@ -363,12 +358,11 @@ TEST_CASE("wait on move constructed active_poller", "[active_poller]")
server_client_setup s;
CHECK_NOTHROW(s.client.send(zmq::message_t{hi_str}, zmq::send_flags::none));
zmq::active_poller_t a;
zmq::active_poller_t::handler_type handler;
CHECK_NOTHROW(a.add(s.server, zmq::event_flags::pollin, handler));
CHECK_NOTHROW(a.add(s.server, zmq::event_flags::pollin, no_op_handler));
zmq::active_poller_t b{std::move(a)};
CHECK(1u == b.size());
/// \todo the actual error code should be checked
CHECK_THROWS_AS(a.wait(std::chrono::milliseconds{10}), const zmq::error_t&);
CHECK(0u == a.size());
CHECK_THROWS_ZMQ_ERROR(EFAULT, a.wait(std::chrono::milliseconds{10}));
CHECK(b.wait(std::chrono::milliseconds{-1}));
}

@@ -377,13 +371,12 @@ TEST_CASE("wait on move assigned active_poller", "[active_poller]")
server_client_setup s;
CHECK_NOTHROW(s.client.send(zmq::message_t{hi_str}, zmq::send_flags::none));
zmq::active_poller_t a;
zmq::active_poller_t::handler_type handler;
CHECK_NOTHROW(a.add(s.server, zmq::event_flags::pollin, handler));
CHECK_NOTHROW(a.add(s.server, zmq::event_flags::pollin, no_op_handler));
zmq::active_poller_t b;
b = {std::move(a)};
CHECK(1u == b.size());
/// \todo the actual error code should be checked
CHECK_THROWS_AS(a.wait(std::chrono::milliseconds{10}), const zmq::error_t&);
CHECK(0u == a.size());
CHECK_THROWS_ZMQ_ERROR(EFAULT, a.wait(std::chrono::milliseconds{10}));
CHECK(b.wait(std::chrono::milliseconds{-1}));
}

@@ -394,9 +387,8 @@ TEST_CASE("received on move constructed active_poller", "[active_poller]")
int count = 0;
// Setup active_poller a
zmq::active_poller_t a;
CHECK_NOTHROW(
a.add(s.server, zmq::event_flags::pollin, [&count](zmq::event_flags) { ++
count; }));
CHECK_NOTHROW(a.add(s.server, zmq::event_flags::pollin,
[&count](zmq::event_flags) { ++count; }));
// client sends message
CHECK_NOTHROW(s.client.send(zmq::message_t{hi_str}, zmq::send_flags::none));
// wait for message and verify it is received
@@ -425,13 +417,13 @@ TEST_CASE("remove from handler", "[active_poller]")
zmq::active_poller_t active_poller;
int count = 0;
for (size_t i = 0; i < ITER_NO; ++i) {
CHECK_NOTHROW(
active_poller.add(setup_list[i].server, zmq::event_flags::pollin, [&, i](
zmq::event_flags events) {
CHECK(events == zmq::event_flags::pollin);
active_poller.remove(setup_list[ITER_NO - i - 1].server);
CHECK((ITER_NO - i - 1) == active_poller.size());
}));
CHECK_NOTHROW(active_poller.add(
setup_list[i].server, zmq::event_flags::pollin,
[&, i](zmq::event_flags events) {
CHECK(events == zmq::event_flags::pollin);
active_poller.remove(setup_list[ITER_NO - i - 1].server);
CHECK((ITER_NO - i - 1) == active_poller.size());
}));
++count;
}
CHECK(ITER_NO == active_poller.size());
19 changes: 19 additions & 0 deletions tests/testutil.hpp
Original file line number Diff line number Diff line change
@@ -33,3 +33,22 @@ struct common_server_client_setup
std::string endpoint;
};
#endif

#define CHECK_THROWS_ZMQ_ERROR(ecode, expr) \
do { \
try { \
expr; \
CHECK(false); \
} \
catch (const zmq::error_t &ze) { \
INFO(std::string("Unexpected error code: ") + ze.what()); \
CHECK(ze.num() == ecode); \
} \
catch (const std::exception &ex) { \
INFO(std::string("Unexpected exception: ") + ex.what()); \
CHECK(false); \
} \
catch (...) { \
CHECK(false); \
} \
} while (false)
Loading

0 comments on commit bf4f75b

Please sign in to comment.