Skip to content

Commit ef37737

Browse files
Pavel OrekhovPavel Orekhov
Pavel Orekhov
authored and
Pavel Orekhov
committed
* fix zeromq#205 "Segmentation at sock.close() at loop.timer".
part 2: exception at loop.start() part 3: socket is not removed from items_ via separate cb for close() * testcases for it
1 parent bd94d1f commit ef37737

File tree

3 files changed

+87
-15
lines changed

3 files changed

+87
-15
lines changed

src/tests/test_loop.cpp

+62
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
#include <thread>
1212
#include <exception>
1313

14+
#define private public
15+
1416
#include "zmqpp/context.hpp"
1517
#include "zmqpp/message.hpp"
1618
#include "zmqpp/loop.hpp"
@@ -72,6 +74,66 @@ BOOST_AUTO_TEST_CASE(socket_removed_in_timer)
7274
BOOST_CHECK(socket_called == false);
7375
}
7476

77+
BOOST_AUTO_TEST_CASE(socket_closed_in_timer)
78+
{
79+
zmqpp::context context;
80+
81+
zmqpp::socket output(context, zmqpp::socket_type::pair);
82+
output.bind("inproc://test");
83+
zmqpp::socket input(context, zmqpp::socket_type::pair);
84+
input.connect("inproc://test");
85+
86+
zmqpp::loop loop;
87+
88+
bool socket_called = false;
89+
90+
loop.add(output, [&socket_called]() -> bool { socket_called = true; return false; });
91+
loop.add(std::chrono::milliseconds(0), 1, [&loop, &output]() -> bool {
92+
loop.remove(output);
93+
output.close();
94+
loop.add(std::chrono::milliseconds(10), 1, []() -> bool { return false; });
95+
return true;
96+
});
97+
98+
input.send("PING");
99+
100+
BOOST_CHECK_NO_THROW(loop.start());
101+
BOOST_CHECK(loop.items_.size() == 0);
102+
BOOST_CHECK(socket_called == false);
103+
}
104+
105+
BOOST_AUTO_TEST_CASE(socket_closed_after_remove_at_timer)
106+
{
107+
zmqpp::context context;
108+
109+
zmqpp::socket output(context, zmqpp::socket_type::pair);
110+
output.bind("inproc://test");
111+
zmqpp::socket input(context, zmqpp::socket_type::pair);
112+
input.connect("inproc://test");
113+
114+
zmqpp::loop loop;
115+
116+
bool socket_called = false;
117+
118+
loop.add(output, [&socket_called]() -> bool { socket_called = true; return false; },
119+
zmqpp::poller::poll_in, [&output]()
120+
{
121+
output.close();
122+
return true;
123+
});
124+
loop.add(std::chrono::milliseconds(0), 1, [&loop, &output]() -> bool {
125+
loop.remove(output);
126+
loop.add(std::chrono::milliseconds(10), 1, []() -> bool { return false; });
127+
return true;
128+
});
129+
130+
input.send("PING");
131+
132+
BOOST_CHECK_NO_THROW(loop.start());
133+
BOOST_CHECK(loop.items_.size() == 0);
134+
BOOST_CHECK(socket_called == false);
135+
}
136+
75137
BOOST_AUTO_TEST_CASE(simple_pull_push)
76138
{
77139
zmqpp::context context;

src/zmqpp/loop.cpp

+19-11
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,10 @@ namespace zmqpp
4747
when += delay;
4848
}
4949

50-
void loop::add(socket& socket, Callable callable, short const event /* = POLL_IN */)
50+
void loop::add(socket& socket, Callable callable, short const event /* = POLL_IN */, Callable after_remove_cb /* = Callable(nullptr)*/)
5151
{
5252
zmq_pollitem_t item{static_cast<void *> (socket), 0, event, 0};
53-
add(item, callable);
53+
add(item, callable, after_remove_cb);
5454
}
5555

5656
void loop::add(raw_socket_t const descriptor, Callable callable, short const event /* = POLL_IN */)
@@ -59,11 +59,11 @@ namespace zmqpp
5959
add(item, callable);
6060
}
6161

62-
void loop::add(const zmq_pollitem_t& item, Callable callable)
62+
void loop::add(const zmq_pollitem_t& item, Callable callable, Callable after_remove_cb)
6363
{
6464
poller_.add(item);
6565
rebuild_poller_ = true;
66-
items_.push_back(std::make_pair(item, callable));
66+
items_.push_back(std::make_tuple(item, callable, after_remove_cb));
6767
}
6868

6969
loop::timer_id_t loop::add(std::chrono::milliseconds delay, size_t times, Callable callable)
@@ -110,16 +110,24 @@ namespace zmqpp
110110
sockRemoveLater_.push_back(&socket);
111111
return;
112112
}
113-
items_.erase(std::remove_if(items_.begin(), items_.end(), [&socket](const PollItemCallablePair & pair) -> bool
113+
114+
std::vector<PollItemCallableTuple> cb_after_remove;
115+
116+
items_.erase(std::remove_if(items_.begin(), items_.end(),
117+
[&socket, &cb_after_remove](const PollItemCallableTuple & tuple) -> bool
114118
{
115-
const zmq_pollitem_t &item = pair.first;
119+
const zmq_pollitem_t &item = std::get<0>(tuple);
116120
if (nullptr != item.socket && item.socket == static_cast<void *> (socket))
117121
{
122+
if(std::get<2>(tuple))
123+
cb_after_remove.push_back(tuple);
118124
return true;
119125
}
120126
return false;
121127
}), items_.end());
122128
poller_.remove(socket);
129+
for (const PollItemCallableTuple& item : cb_after_remove)
130+
std::get<2>(item)();
123131
}
124132

125133
void loop::remove(raw_socket_t const descriptor)
@@ -130,9 +138,9 @@ namespace zmqpp
130138
fdRemoveLater_.push_back(descriptor);
131139
return;
132140
}
133-
items_.erase(std::remove_if(items_.begin(), items_.end(), [descriptor](const PollItemCallablePair & pair) -> bool
141+
items_.erase(std::remove_if(items_.begin(), items_.end(), [descriptor](const PollItemCallableTuple & tuple) -> bool
134142
{
135-
const zmq_pollitem_t &item = pair.first;
143+
const zmq_pollitem_t &item = std::get<0>(tuple);
136144
if (nullptr == item.socket && item.fd == descriptor)
137145
{
138146
return true;
@@ -194,12 +202,12 @@ namespace zmqpp
194202

195203
bool loop::start_handle_poller()
196204
{
197-
for (const PollItemCallablePair &pair : items_)
205+
for (const PollItemCallableTuple &tuple : items_)
198206
{
199-
const zmq_pollitem_t &pollitem = pair.first;
207+
const zmq_pollitem_t &pollitem = std::get<0>(tuple);
200208

201209
if (poller_.has_input(pollitem) || poller_.has_error(pollitem) || poller_.has_output(pollitem))
202-
if(!pair.second())
210+
if(!std::get<1>(tuple)())
203211
return false;
204212
}
205213
return true;

src/zmqpp/loop.hpp

+6-4
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,10 @@ namespace zmqpp
5959
* \param socket the socket to monitor.
6060
* \param callable the function that will be called by the loop when a registered event occurs on socket.
6161
* \param event the event flags to monitor on the socket.
62+
* \param after_remove_cb will be called by loop after remove() completion.
63+
* See tests/test_loop.cpp: socket_closed_in_timer and socket_closed_after_remove_at_timer
6264
*/
63-
void add(socket_t& socket, Callable callable, short const event = poller::poll_in);
65+
void add(socket_t& socket, Callable callable, short const event = poller::poll_in, Callable after_remove_cb = Callable(nullptr));
6466

6567
/*!
6668
* Add a standard socket to the loop, providing a handler that will be called when the monitored events occur.
@@ -125,18 +127,18 @@ namespace zmqpp
125127
void update();
126128
};
127129

128-
typedef std::pair<zmq_pollitem_t, Callable> PollItemCallablePair;
130+
typedef std::tuple<zmq_pollitem_t, Callable, Callable> PollItemCallableTuple;
129131
typedef std::pair<std::unique_ptr<timer_t>, Callable> TimerItemCallablePair;
130132
static bool TimerItemCallablePairComp(const TimerItemCallablePair &lhs, const TimerItemCallablePair &rhs);
131133

132-
std::vector<PollItemCallablePair> items_;
134+
std::vector<PollItemCallableTuple> items_;
133135
std::list<TimerItemCallablePair> timers_;
134136
std::vector<const socket_t *> sockRemoveLater_;
135137
std::vector<raw_socket_t> fdRemoveLater_;
136138
std::vector<timer_id_t> timerRemoveLater_;
137139

138140

139-
void add(const zmq_pollitem_t &item, Callable callable);
141+
void add(const zmq_pollitem_t &item, Callable callable, Callable after_remove_cb = Callable(nullptr));
140142
void add(std::unique_ptr<timer_t>, Callable callable);
141143

142144
bool start_handle_timers();

0 commit comments

Comments
 (0)