Skip to content

Commit 810b87c

Browse files
committed
Problem: poller_t invalid behaviour on invalid sockets
On adding invalid socket (e.g. after move) there was exception thrown but leaving modified and unconsistent internal state. Besides that there was no possibility to remove a socket that was moved into. Solutions: check for socket validity (added operator bool) and changed internal unordered_map "handlers" to operator on zmq internal pointers. Added two test cases covering the issues.
1 parent 60f7753 commit 810b87c

File tree

2 files changed

+34
-6
lines changed

2 files changed

+34
-6
lines changed

tests/poller.cpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,4 +228,28 @@ TEST(poller, client_server)
228228
ASSERT_TRUE(got_pollout);
229229
}
230230

231+
TEST(poller, poller_add_invalid_socket_throws)
232+
{
233+
zmq::context_t context;
234+
zmq::poller_t poller;
235+
zmq::socket_t a {context, zmq::socket_type::router};
236+
zmq::socket_t b {std::move (a)};
237+
ASSERT_THROW (poller.add (a, ZMQ_POLLIN, zmq::poller_t::handler_t {}),
238+
zmq::error_t);
239+
ASSERT_EQ (0u, poller.size ());
240+
}
241+
242+
TEST(poller, poller_remove_invalid_socket_throws)
243+
{
244+
zmq::context_t context;
245+
zmq::socket_t socket {context, zmq::socket_type::router};
246+
zmq::poller_t poller;
247+
ASSERT_NO_THROW (poller.add (socket, ZMQ_POLLIN, zmq::poller_t::handler_t {}));
248+
ASSERT_EQ (1u, poller.size ());
249+
std::vector<zmq::socket_t> sockets;
250+
sockets.emplace_back (std::move (socket));
251+
ASSERT_THROW (poller.remove (socket), zmq::error_t);
252+
ASSERT_EQ (1u, poller.size ());
253+
}
254+
231255
#endif

zmq.hpp

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,11 @@ namespace zmq
536536
{
537537
return ptr;
538538
}
539+
540+
inline operator bool() const ZMQ_NOTHROW
541+
{
542+
return ptr != NULL;
543+
}
539544
private:
540545

541546
void *ptr;
@@ -1053,10 +1058,12 @@ namespace zmq
10531058

10541059
void add (zmq::socket_t &socket, short events, handler_t handler)
10551060
{
1061+
if (!socket)
1062+
throw error_t ();
10561063
handler_t *handler_ptr = nullptr;
10571064
/// \todo is it sensible to allow handler to be empty? doesn't this lead to an error when the event is signalled? (perhaps it should already lead to an error in zmq_poller_add then)
10581065
if (handler) {
1059-
auto emplace_res = handlers.emplace(&socket, std::move(handler));
1066+
auto emplace_res = handlers.emplace (socket.ptr, std::move (handler));
10601067
handler_ptr = &emplace_res.first->second;
10611068
}
10621069
if (0 == zmq_poller_add (poller_ptr, socket.ptr, handler_ptr, events)) {
@@ -1069,10 +1076,7 @@ namespace zmq
10691076
void remove (zmq::socket_t &socket)
10701077
{
10711078
if (0 == zmq_poller_remove (poller_ptr, socket.ptr)) {
1072-
auto it = handlers.find (&socket);
1073-
if (it != handlers.end ()) { /// \todo this may only be false if handler was empty on add
1074-
handlers.erase (it);
1075-
}
1079+
handlers.erase (socket.ptr);
10761080
poller_events.pop_back ();
10771081
return;
10781082
}
@@ -1108,7 +1112,7 @@ namespace zmq
11081112
private:
11091113
void *poller_ptr;
11101114
std::vector<zmq_poller_event_t> poller_events;
1111-
std::unordered_map<socket_t*, handler_t> handlers;
1115+
std::unordered_map<void*, handler_t> handlers;
11121116
}; // class poller_t
11131117
#endif // defined(ZMQ_BUILD_DRAFT_API) && defined(ZMQ_CPP11) && defined(ZMQ_HAVE_POLLER)
11141118

0 commit comments

Comments
 (0)