Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use executor in dtls_acceptor_callback_helper #4

Merged
merged 4 commits into from
Oct 22, 2018

Conversation

sdamm
Copy link
Owner

@sdamm sdamm commented Sep 3, 2018

No description provided.

@@ -767,6 +768,7 @@ class acceptor
, ah_(std::move(ah))
, sock_(sock)
, buffer_(buffer)
, work_(associated_executor<AcceptHandler>::get(ah))
Copy link

@vinniefalco vinniefalco Sep 3, 2018

Choose a reason for hiding this comment

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

hmm...no, I don't think this is right. The work should be using the executor of the I/O object and not the executor associated with the completion handler. The completion handler's executor will automatically get a work guard when you submit your own composed operation to an Asio I/O operation (I think you are calling the datagram socket's async_receive_from?)

Asio (and therefore Networking TS) require two executor work guards. One of them you get for "free" when you call an I/O operation, and the other one you have to provide yourself.

Example:
https://github.com/boostorg/beast/blob/1da229a27c6f0539d422bcedbcf47cfe2517164a/include/boost/beast/http/impl/read.ipp#L63

@@ -767,6 +768,7 @@ class acceptor
, ah_(std::move(ah))

Choose a reason for hiding this comment

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

I think you want to be using std::forward in a deduced context, example:
https://github.com/boostorg/beast/blob/1da229a27c6f0539d422bcedbcf47cfe2517164a/include/boost/beast/http/impl/read.ipp#L66

You really should study Beast and the composed operation tutorial! This is a very complex topic and it is super easy to make mistakes (happens to me all the time).

using executor_type = asio::associated_executor_t<
AcceptHandler, decltype(std::declval<DatagramSocketType&>().get_executor())>;

using executor_type = typename associated_executor<AcceptHandler>::type;

Choose a reason for hiding this comment

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

Readme.md Outdated
A dtls\_context allows to use methos like dtls\_client which a normal ssl::context
does not. A dtls\_context does not allow stream based methos like tlsv12\_server.


Choose a reason for hiding this comment

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

Very helpful!!

@@ -282,7 +280,7 @@ class acceptor
ASIO_SYNC_OP_VOID set_option(const SettableSocketOption& option,
asio::error_code& ec)
{
this->get_service().set_option(this->get_implementation(), option, ec);

Choose a reason for hiding this comment

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

This is much better!

@@ -42,6 +42,10 @@ class DTLS_Server
: m_acceptor(serv, ep)
, ctx_(ctx)
{
asio::socket_base::reuse_address option(true);
m_acceptor.set_option(option);
Copy link

@vinniefalco vinniefalco Sep 11, 2018

Choose a reason for hiding this comment

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

Consider:

m_acceptor.set_option(asio::socket_base::reuse_address option(true));

@sdamm sdamm merged commit ce89dd6 into master Oct 22, 2018
@sdamm sdamm deleted the add_executor_for_acceptor branch October 22, 2018 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants