Skip to content

Commit bf4f75b

Browse files
sigiesecsigiesec
authored
Merge pull request #405 from gummif/gfa/active-poller-handler
Problem: Active poller double add mutates handler
2 parents 7efc9b1 + d237615 commit bf4f75b

File tree

4 files changed

+111
-100
lines changed

4 files changed

+111
-100
lines changed

‎tests/active_poller.cpp

+78-86
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,12 @@ TEST_CASE("create destroy", "[active_poller]")
1414
}
1515

1616
static_assert(!std::is_copy_constructible<zmq::active_poller_t>::value,
17-
"active_active_poller_t should not be copy-constructible");
17+
"active_poller_t should not be copy-constructible");
1818
static_assert(!std::is_copy_assignable<zmq::active_poller_t>::value,
19-
"active_active_poller_t should not be copy-assignable");
19+
"active_poller_t should not be copy-assignable");
20+
21+
static const zmq::active_poller_t::handler_type no_op_handler =
22+
[](zmq::event_flags) {};
2023

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

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

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

8180
TEST_CASE("add handler", "[active_poller]")
81+
{
82+
zmq::context_t context;
83+
zmq::socket_t socket{context, zmq::socket_type::router};
84+
zmq::active_poller_t active_poller;
85+
CHECK_NOTHROW(
86+
active_poller.add(socket, zmq::event_flags::pollin, no_op_handler));
87+
}
88+
89+
TEST_CASE("add null handler fails", "[active_poller]")
8290
{
8391
zmq::context_t context;
8492
zmq::socket_t socket{context, zmq::socket_type::router};
8593
zmq::active_poller_t active_poller;
8694
zmq::active_poller_t::handler_type handler;
87-
CHECK_NOTHROW(active_poller.add(socket, zmq::event_flags::pollin, handler));
95+
CHECK_THROWS_AS(active_poller.add(socket, zmq::event_flags::pollin, handler),
96+
const std::invalid_argument &);
8897
}
8998

9099
#if ZMQ_VERSION >= ZMQ_MAKE_VERSION(4, 3, 0)
@@ -94,52 +103,54 @@ TEST_CASE("add handler invalid events type", "[active_poller]")
94103
zmq::context_t context;
95104
zmq::socket_t socket{context, zmq::socket_type::router};
96105
zmq::active_poller_t active_poller;
97-
zmq::active_poller_t::handler_type handler;
98106
short invalid_events_type = 2 << 10;
99107
CHECK_THROWS_AS(
100-
active_poller.add(socket, static_cast<zmq::event_flags>(invalid_events_type),
101-
handler), const zmq::error_t&);
108+
active_poller.add(socket, static_cast<zmq::event_flags>(invalid_events_type),
109+
no_op_handler),
110+
const zmq::error_t &);
102111
CHECK(active_poller.empty());
103112
CHECK(0u == active_poller.size());
104113
}
105114
#endif
106115

107116
TEST_CASE("add handler twice throws", "[active_poller]")
108117
{
109-
zmq::context_t context;
110-
zmq::socket_t socket{context, zmq::socket_type::router};
118+
common_server_client_setup s;
119+
120+
CHECK(s.client.send(zmq::message_t{}, zmq::send_flags::none));
121+
111122
zmq::active_poller_t active_poller;
112-
zmq::active_poller_t::handler_type handler;
113-
active_poller.add(socket, zmq::event_flags::pollin, handler);
114-
/// \todo the actual error code should be checked
115-
CHECK_THROWS_AS(active_poller.add(socket, zmq::event_flags::pollin, handler),
116-
const zmq::error_t&);
123+
bool message_received = false;
124+
active_poller.add(
125+
s.server, zmq::event_flags::pollin,
126+
[&message_received](zmq::event_flags) { message_received = true; });
127+
CHECK_THROWS_ZMQ_ERROR(
128+
EINVAL, active_poller.add(s.server, zmq::event_flags::pollin, no_op_handler));
129+
CHECK(1 == active_poller.wait(std::chrono::milliseconds{-1}));
130+
CHECK(message_received); // handler unmodified
117131
}
118132

119133
TEST_CASE("wait with no handlers throws", "[active_poller]")
120134
{
121135
zmq::active_poller_t active_poller;
122-
/// \todo the actual error code should be checked
123-
CHECK_THROWS_AS(active_poller.wait(std::chrono::milliseconds{10}),
124-
const zmq::error_t&);
136+
CHECK_THROWS_ZMQ_ERROR(EFAULT,
137+
active_poller.wait(std::chrono::milliseconds{10}));
125138
}
126139

127140
TEST_CASE("remove unregistered throws", "[active_poller]")
128141
{
129142
zmq::context_t context;
130143
zmq::socket_t socket{context, zmq::socket_type::router};
131144
zmq::active_poller_t active_poller;
132-
/// \todo the actual error code should be checked
133-
CHECK_THROWS_AS(active_poller.remove(socket), const zmq::error_t&);
145+
CHECK_THROWS_ZMQ_ERROR(EINVAL, active_poller.remove(socket));
134146
}
135147

136148
TEST_CASE("remove registered empty", "[active_poller]")
137149
{
138150
zmq::context_t context;
139151
zmq::socket_t socket{context, zmq::socket_type::router};
140152
zmq::active_poller_t active_poller;
141-
active_poller.add(socket, zmq::event_flags::pollin,
142-
zmq::active_poller_t::handler_type{});
153+
active_poller.add(socket, zmq::event_flags::pollin, no_op_handler);
143154
CHECK_NOTHROW(active_poller.remove(socket));
144155
}
145156

@@ -148,18 +159,15 @@ TEST_CASE("remove registered non empty", "[active_poller]")
148159
zmq::context_t context;
149160
zmq::socket_t socket{context, zmq::socket_type::router};
150161
zmq::active_poller_t active_poller;
151-
active_poller.add(socket, zmq::event_flags::pollin, [](zmq::event_flags)
152-
{
153-
});
162+
active_poller.add(socket, zmq::event_flags::pollin, no_op_handler);
154163
CHECK_NOTHROW(active_poller.remove(socket));
155164
}
156165

157166
namespace
158167
{
159168
struct server_client_setup : common_server_client_setup
160169
{
161-
zmq::active_poller_t::handler_type handler = [&](zmq::event_flags e)
162-
{
170+
zmq::active_poller_t::handler_type handler = [&](zmq::event_flags e) {
163171
events = e;
164172
};
165173

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

179187
zmq::active_poller_t active_poller;
180188
bool message_received = false;
181-
zmq::active_poller_t::handler_type handler = [&message_received
182-
](zmq::event_flags events)
183-
{
184-
CHECK(zmq::event_flags::none != (events & zmq::event_flags::pollin));
185-
message_received = true;
186-
};
189+
zmq::active_poller_t::handler_type handler =
190+
[&message_received](zmq::event_flags events) {
191+
CHECK(zmq::event_flags::none != (events & zmq::event_flags::pollin));
192+
message_received = true;
193+
};
187194
CHECK_NOTHROW(active_poller.add(s.server, zmq::event_flags::pollin, handler));
188195
CHECK(1 == active_poller.wait(std::chrono::milliseconds{-1}));
189196
CHECK(message_received);
@@ -200,8 +207,7 @@ TEST_CASE("client server", "[active_poller]")
200207
// Setup active_poller
201208
zmq::active_poller_t active_poller;
202209
zmq::event_flags events;
203-
zmq::active_poller_t::handler_type handler = [&](zmq::event_flags e)
204-
{
210+
zmq::active_poller_t::handler_type handler = [&](zmq::event_flags e) {
205211
if (zmq::event_flags::none != (e & zmq::event_flags::pollin)) {
206212
zmq::message_t zmq_msg;
207213
CHECK_NOTHROW(s.server.recv(zmq_msg)); // get message
@@ -224,9 +230,8 @@ TEST_CASE("client server", "[active_poller]")
224230

225231
// Re-add server socket with pollout flag
226232
CHECK_NOTHROW(active_poller.remove(s.server));
227-
CHECK_NOTHROW(
228-
active_poller.add(s.server, zmq::event_flags::pollin | zmq::event_flags::
229-
pollout, handler));
233+
CHECK_NOTHROW(active_poller.add(
234+
s.server, zmq::event_flags::pollin | zmq::event_flags::pollout, handler));
230235
CHECK(1 == active_poller.wait(std::chrono::milliseconds{-1}));
231236
CHECK(events == zmq::event_flags::pollout);
232237
}
@@ -237,10 +242,8 @@ TEST_CASE("add invalid socket throws", "[active_poller]")
237242
zmq::active_poller_t active_poller;
238243
zmq::socket_t a{context, zmq::socket_type::router};
239244
zmq::socket_t b{std::move(a)};
240-
CHECK_THROWS_AS(
241-
active_poller.add(a, zmq::event_flags::pollin, zmq::active_poller_t::
242-
handler_type{}),
243-
const zmq::error_t&);
245+
CHECK_THROWS_AS(active_poller.add(a, zmq::event_flags::pollin, no_op_handler),
246+
const zmq::error_t &);
244247
}
245248

246249
TEST_CASE("remove invalid socket throws", "[active_poller]")
@@ -249,12 +252,11 @@ TEST_CASE("remove invalid socket throws", "[active_poller]")
249252
zmq::socket_t socket{context, zmq::socket_type::router};
250253
zmq::active_poller_t active_poller;
251254
CHECK_NOTHROW(
252-
active_poller.add(socket, zmq::event_flags::pollin, zmq::active_poller_t::
253-
handler_type{}));
255+
active_poller.add(socket, zmq::event_flags::pollin, no_op_handler));
254256
CHECK(1u == active_poller.size());
255257
std::vector<zmq::socket_t> sockets;
256258
sockets.emplace_back(std::move(socket));
257-
CHECK_THROWS_AS(active_poller.remove(socket), const zmq::error_t&);
259+
CHECK_THROWS_AS(active_poller.remove(socket), const zmq::error_t &);
258260
CHECK(1u == active_poller.size());
259261
}
260262

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

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

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

290292
TEST_CASE("modify not added throws", "[active_poller]")
@@ -293,24 +295,19 @@ TEST_CASE("modify not added throws", "[active_poller]")
293295
zmq::socket_t a{context, zmq::socket_type::push};
294296
zmq::socket_t b{context, zmq::socket_type::push};
295297
zmq::active_poller_t active_poller;
296-
CHECK_NOTHROW(
297-
active_poller.add(a, zmq::event_flags::pollin, zmq::active_poller_t::
298-
handler_type{}));
298+
CHECK_NOTHROW(active_poller.add(a, zmq::event_flags::pollin, no_op_handler));
299299
CHECK_THROWS_AS(active_poller.modify(b, zmq::event_flags::pollin),
300-
const zmq::error_t&);
300+
const zmq::error_t &);
301301
}
302302

303303
TEST_CASE("modify simple", "[active_poller]")
304304
{
305305
zmq::context_t context;
306306
zmq::socket_t a{context, zmq::socket_type::push};
307307
zmq::active_poller_t active_poller;
308+
CHECK_NOTHROW(active_poller.add(a, zmq::event_flags::pollin, no_op_handler));
308309
CHECK_NOTHROW(
309-
active_poller.add(a, zmq::event_flags::pollin, zmq::active_poller_t::
310-
handler_type{}));
311-
CHECK_NOTHROW(
312-
active_poller.modify(a, zmq::event_flags::pollin | zmq::event_flags::pollout
313-
));
310+
active_poller.modify(a, zmq::event_flags::pollin | zmq::event_flags::pollout));
314311
}
315312

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

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

347343
// Setup active_poller
348344
zmq::active_poller_t active_poller;
349-
CHECK_NOTHROW(
350-
active_poller.add(s.server, zmq::event_flags::pollin, [&count](zmq::
351-
event_flags) { ++count; }));
345+
CHECK_NOTHROW(active_poller.add(s.server, zmq::event_flags::pollin,
346+
[&count](zmq::event_flags) { ++count; }));
352347

353348
// client sends message
354349
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]")
363358
server_client_setup s;
364359
CHECK_NOTHROW(s.client.send(zmq::message_t{hi_str}, zmq::send_flags::none));
365360
zmq::active_poller_t a;
366-
zmq::active_poller_t::handler_type handler;
367-
CHECK_NOTHROW(a.add(s.server, zmq::event_flags::pollin, handler));
361+
CHECK_NOTHROW(a.add(s.server, zmq::event_flags::pollin, no_op_handler));
368362
zmq::active_poller_t b{std::move(a)};
369363
CHECK(1u == b.size());
370-
/// \todo the actual error code should be checked
371-
CHECK_THROWS_AS(a.wait(std::chrono::milliseconds{10}), const zmq::error_t&);
364+
CHECK(0u == a.size());
365+
CHECK_THROWS_ZMQ_ERROR(EFAULT, a.wait(std::chrono::milliseconds{10}));
372366
CHECK(b.wait(std::chrono::milliseconds{-1}));
373367
}
374368

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

@@ -394,9 +387,8 @@ TEST_CASE("received on move constructed active_poller", "[active_poller]")
394387
int count = 0;
395388
// Setup active_poller a
396389
zmq::active_poller_t a;
397-
CHECK_NOTHROW(
398-
a.add(s.server, zmq::event_flags::pollin, [&count](zmq::event_flags) { ++
399-
count; }));
390+
CHECK_NOTHROW(a.add(s.server, zmq::event_flags::pollin,
391+
[&count](zmq::event_flags) { ++count; }));
400392
// client sends message
401393
CHECK_NOTHROW(s.client.send(zmq::message_t{hi_str}, zmq::send_flags::none));
402394
// wait for message and verify it is received
@@ -425,13 +417,13 @@ TEST_CASE("remove from handler", "[active_poller]")
425417
zmq::active_poller_t active_poller;
426418
int count = 0;
427419
for (size_t i = 0; i < ITER_NO; ++i) {
428-
CHECK_NOTHROW(
429-
active_poller.add(setup_list[i].server, zmq::event_flags::pollin, [&, i](
430-
zmq::event_flags events) {
431-
CHECK(events == zmq::event_flags::pollin);
432-
active_poller.remove(setup_list[ITER_NO - i - 1].server);
433-
CHECK((ITER_NO - i - 1) == active_poller.size());
434-
}));
420+
CHECK_NOTHROW(active_poller.add(
421+
setup_list[i].server, zmq::event_flags::pollin,
422+
[&, i](zmq::event_flags events) {
423+
CHECK(events == zmq::event_flags::pollin);
424+
active_poller.remove(setup_list[ITER_NO - i - 1].server);
425+
CHECK((ITER_NO - i - 1) == active_poller.size());
426+
}));
435427
++count;
436428
}
437429
CHECK(ITER_NO == active_poller.size());

‎tests/testutil.hpp

+19
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,22 @@ struct common_server_client_setup
3333
std::string endpoint;
3434
};
3535
#endif
36+
37+
#define CHECK_THROWS_ZMQ_ERROR(ecode, expr) \
38+
do { \
39+
try { \
40+
expr; \
41+
CHECK(false); \
42+
} \
43+
catch (const zmq::error_t &ze) { \
44+
INFO(std::string("Unexpected error code: ") + ze.what()); \
45+
CHECK(ze.num() == ecode); \
46+
} \
47+
catch (const std::exception &ex) { \
48+
INFO(std::string("Unexpected exception: ") + ex.what()); \
49+
CHECK(false); \
50+
} \
51+
catch (...) { \
52+
CHECK(false); \
53+
} \
54+
} while (false)

0 commit comments

Comments
 (0)