Skip to content

Commit

Permalink
Remove flakiness and improve coverage.
Browse files Browse the repository at this point in the history
  • Loading branch information
coryan committed Jun 19, 2017
1 parent 17e5152 commit 43e9cd1
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 2 deletions.
1 change: 1 addition & 0 deletions jb/ehs/acceptor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ void acceptor::on_accept(boost::system::error_code const& ec) {
// to shutdown the application, so we simply return and do not
// schedule any additional work ...
JB_LOG(info) << "on_accept: acceptor is not open";
dispatcher_->count_accept_closed();
return;
}
// ... move the newly created socket to a stack variable so we can
Expand Down
7 changes: 6 additions & 1 deletion jb/ehs/request_dispatcher.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ request_dispatcher::request_dispatcher(std::string const& server_name)
, write_ok_(0)
, write_error_(0)
, accept_ok_(0)
, accept_error_(0) {
, accept_error_(0)
, accept_closed_(0) {
#ifndef ATOMIC_LONG_LOCK_FREE
#error "Missing ATOMIC_LONG_LOCK_FREE required by C++11 standard"
#endif // ATOMIC_LONG_LOCK_FREE
Expand Down Expand Up @@ -105,6 +106,10 @@ void request_dispatcher::append_metrics(response_type& res) const {
<< "# HELP accept_error The number of errors accepting HTTP connections\n"
<< "# TYPE accept_error counter\n"
<< "accept_error " << get_accept_error() << "\n"
<< "# HELP accept_closed The number accept() attempts on a closed "
"acceptor\n"
<< "# TYPE accept_closed counter\n"
<< "accept_closed " << get_accept_closed() << "\n"
<< "\n";
res.body += os.str();
}
Expand Down
9 changes: 9 additions & 0 deletions jb/ehs/request_dispatcher.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,14 @@ class request_dispatcher {
long get_accept_error() const {
return accept_error_;
}

/// Count accept requests on closed acceptors (rare)
void count_accept_closed() {
++accept_closed_;
}
long get_accept_closed() const {
return accept_closed_;
}
//@}

/**
Expand Down Expand Up @@ -258,6 +266,7 @@ class request_dispatcher {
std::atomic<long> write_error_;
std::atomic<long> accept_ok_;
std::atomic<long> accept_error_;
std::atomic<long> accept_closed_;
};

} // namespace ehs
Expand Down
39 changes: 38 additions & 1 deletion jb/ehs/ut_acceptor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace {
void wait_for_connection_close(
std::shared_ptr<jb::ehs::request_dispatcher> d, long last_count) {
// .. wait until the connection is closed ...
for (int c = 0; c != 10 and last_count == d->get_close_connection(); ++c) {
for (int c = 0; c != 100 and last_count == d->get_close_connection(); ++c) {
std::this_thread::sleep_for(std::chrono::milliseconds(10));
}
BOOST_CHECK_EQUAL(d->get_close_connection(), last_count + 1);
Expand Down Expand Up @@ -74,6 +74,7 @@ BOOST_AUTO_TEST_CASE(acceptor_base) {
// ... closing the socket triggers more behaviors in the acceptor
// and connector classes ...
sock.close();
BOOST_TEST_CHECKPOINT("closing connection in acceptor_base");
wait_for_connection_close(dispatcher, dispatcher->get_close_connection());

// ... shutdown the acceptor and stop the io_service ...
Expand Down Expand Up @@ -183,6 +184,7 @@ void cycle_connection(

// ... close the socket ...
sock.close();
BOOST_TEST_CHECKPOINT("closing connection in cycle_connection");
wait_for_connection_close(d, close_count);
}

Expand Down Expand Up @@ -279,6 +281,7 @@ BOOST_AUTO_TEST_CASE(connection_multiple_requests) {
// and connector classes ...
auto close_count = dispatcher->get_close_connection();
sock.close();
BOOST_TEST_CHECKPOINT("closing connection in connection_multiple_requests");
wait_for_connection_close(dispatcher, close_count);

// ... shutdown the acceptor and stop the io_service ...
Expand Down Expand Up @@ -399,3 +402,37 @@ BOOST_AUTO_TEST_CASE(acceptor_double_shutdown) {

BOOST_CHECK_EQUAL(dispatcher->get_accept_error(), 0);
}

/**
* @test Improve coverage forjb::ehs::acceptor.
*/
BOOST_AUTO_TEST_CASE(acceptor_on_accept_closed) {
// Let the operating system pick a listening address ...
boost::asio::ip::tcp::endpoint ep{boost::asio::ip::address_v4(), 0};

// ... create a dispatcher, with no handlers ...
auto dispatcher = std::make_shared<jb::ehs::request_dispatcher>("test");
// ... create a IO service, and an acceptor in the default address ...
boost::asio::io_service io_service;

// ... the acceptor will register for the on_accept() callback ...
jb::ehs::acceptor acceptor(io_service, ep, dispatcher);

// ... close the acceptor before it has a chance to do anything ...
acceptor.shutdown();

// ... the thread will call on_accept() just because it is closed,
// and that will hit one more path ...
std::thread t([&io_service]() { io_service.run(); });

for (int c = 0; c != 100 and 0 == dispatcher->get_accept_closed(); ++c) {
std::this_thread::sleep_for(std::chrono::milliseconds(10));
}
BOOST_CHECK_EQUAL(dispatcher->get_accept_closed(), 1);

// ... shutdown everything ...
io_service.dispatch([&acceptor, &io_service] {
io_service.stop();
});
t.join();
}

0 comments on commit 43e9cd1

Please sign in to comment.