Skip to content

Commit

Permalink
Close acceptor if it is open
Browse files Browse the repository at this point in the history
  • Loading branch information
@lamyj
lamyj committed Aug 15, 2016
1 parent e099640 commit 514c8b6
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 7 deletions.
23 changes: 16 additions & 7 deletions src/odil/dul/Transport.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,11 @@ ::receive(Socket::endpoint_type const & endpoint)
this->_start_deadline(source, error);

this->_socket = std::make_shared<Socket>(this->_service);
boost::asio::ip::tcp::acceptor acceptor(this->_service, endpoint);
acceptor.async_accept(
this->_acceptor = std::make_shared<boost::asio::ip::tcp::acceptor>(
this->_service, endpoint);
boost::asio::socket_base::reuse_address option(true);

This comment has been minimized.

Sign in to view

Copy link
@cguebert

cguebert Oct 19, 2016

Contributor

I just had the funniest bug... I was only receiving half the connections I was creating on the other end.
It appeared I launched my program twice, and with the option reuse_address to true, each program was sharing the same listening port.
Not sure it is a good idea?

This comment has been minimized.

Sign in to view

Copy link
@lamyj

lamyj Oct 20, 2016

Author Owner

Actually, without this option I got the exact opposite of your bug: sockets not being closed cleanly with no new incoming connection accepted within a 30 seconds time window.

Created issue #41.
this->_acceptor->set_option(option);
this->_acceptor->async_accept(
*this->_socket,
[&source,&error](boost::system::error_code const & e)
{
Expand All @@ -139,19 +142,25 @@ ::receive(Socket::endpoint_type const & endpoint)
);

this->_run(source, error);

this->_acceptor = nullptr;
}

void
Transport
::close()
{
if(!this->is_open())
if(this->_acceptor && this->_acceptor->is_open())
{
throw Exception("Not connected");
this->_acceptor->close();
this->_acceptor = nullptr;
}
if(this->is_open())
{
this->_socket->shutdown(boost::asio::ip::tcp::socket::shutdown_both);
this->_socket->close();
this->_socket = nullptr;
}

this->_socket->close();
this->_socket = nullptr;
}

std::string
Expand Down
2 changes: 2 additions & 0 deletions src/odil/dul/Transport.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ struct Transport
duration_type _timeout;
boost::asio::deadline_timer _deadline;

std::shared_ptr<boost::asio::ip::tcp::acceptor> _acceptor;

enum class Source
{
NONE,
Expand Down

1 comment on commit 514c8b6

@cguebert
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On windows at least, calling shutdown on an opened but not connected socket throws an exception. It occurs in the destructor of dul::Transport, so it crashes the application with no hope of recovery.

It happened when I tried to connect using the wrong host port. Association::associate threw, I catched it, then the Association was freed and down the line the socket threw.

I am not sure how to protect against these kind of network errors, but we really ought to solidify odil in this area.

Please sign in to comment.