review: use asio stackless coroutines instead of expensive template instantiations

5 files changed, 124 insertions, 89 deletions

diff --git a/src/lib/tls/asio/asio_async_base.h b/src/lib/tls/asio/asio_async_base.h
index f476f7155..e970663bd 100644
--- a/src/lib/tls/asio/asio_async_base.h
+++ b/src/lib/tls/asio/asio_async_base.h
@@ -11,6 +11,7 @@
 
 #include <boost/beast/core/bind_handler.hpp>
 
+#include <boost/asio/coroutine.hpp>
 #include <botan/internal/asio_includes.h>
 
 namespace Botan {
@@ -18,7 +19,7 @@ namespace Botan {
 namespace TLS {
 
 template <class Handler, class Executor1, class Allocator>
-struct AsyncBase
+struct AsyncBase : boost::asio::coroutine
    {
       using allocator_type = boost::asio::associated_allocator_t<Handler, Allocator>;
       using executor_type = boost::asio::associated_executor_t<Handler, Executor1>;
@@ -42,22 +43,10 @@ struct AsyncBase
          }
 
       template<class... Args>
-      void invoke(bool isContinuation, Args&& ... args)
+      void invoke_now(Args&& ... args)
          {
-         if(!isContinuation)
-            {
-            // \note(toesterreich): Is this ok to do with bind_handler? Do we need placeholders?
-            boost::asio::post(boost::asio::bind_executor(
-                                 m_work_guard_1.get_executor(), boost::beast::bind_handler(std::move(m_handler), args...))
-                             );
-
-            m_work_guard_1.reset();
-            }
-         else
-            {
-            m_handler(std::forward<Args>(args)...);
-            m_work_guard_1.reset();
-            }
+         m_handler(std::forward<Args>(args)...);
+         m_work_guard_1.reset();
          }
 
       Handler m_handler;
diff --git a/src/lib/tls/asio/asio_async_handshake_op.h b/src/lib/tls/asio/asio_async_handshake_op.h
index 18e368603..406781e47 100644
--- a/src/lib/tls/asio/asio_async_handshake_op.h
+++ b/src/lib/tls/asio/asio_async_handshake_op.h
@@ -11,8 +11,10 @@
 
 #include <botan/internal/asio_async_write_op.h>
 #include <botan/internal/asio_convert_exceptions.h>
-#include <botan/internal/asio_stream_core.h>
 #include <botan/internal/asio_includes.h>
+#include <botan/internal/asio_stream_core.h>
+
+#include <boost/asio/yield.hpp>
 
 namespace Botan {
 
@@ -41,55 +43,70 @@ struct AsyncHandshakeOperation : public AsyncBase<Handler, typename Stream::exec
 
       void operator()(boost::system::error_code ec, std::size_t bytesTransferred, bool isContinuation = true)
          {
-         // process tls packets from socket first
-         if(bytesTransferred > 0)
+         m_ec = ec;
+         reenter(this)
             {
-            boost::asio::const_buffer read_buffer {m_core.input_buffer.data(), bytesTransferred};
-            try
+            // process tls packets from socket first
+
+            if(bytesTransferred > 0)
                {
-               m_stream.native_handle()->received_data(static_cast<const uint8_t*>(read_buffer.data()), read_buffer.size());
+               boost::asio::const_buffer read_buffer {m_core.input_buffer.data(), bytesTransferred};
+               try
+                  {
+                  m_stream.native_handle()->received_data(static_cast<const uint8_t*>(read_buffer.data()), read_buffer.size());
+                  }
+               catch(const std::exception&)
+                  {
+                  m_ec = convertException();
+                  }
                }
-            catch(const std::exception&)
+
+            // send tls packets
+            if(m_core.hasDataToSend() && !m_ec)
                {
-               ec = convertException();
-               this->invoke(isContinuation, ec);
+               // \note: we construct `AsyncWriteOperation` with 0 as its last parameter (`plainBytesTransferred`).
+               //        This operation will eventually call `*this` as its own handler, passing the 0 back to this call
+               //        operator. This is necessary because, the check of `bytesTransferred > 0` assumes that
+               //        `bytesTransferred` bytes were just read and are in the cores input_buffer for further processing.
+               AsyncWriteOperation<
+               AsyncHandshakeOperation<typename std::decay<Handler>::type, Stream, Allocator>,
+                                       Stream,
+                                       Allocator>
+                                       op{std::move(*this), m_stream, m_core, 0};
+               boost::asio::async_write(m_stream.next_layer(), m_core.sendBuffer(), std::move(op));
                return;
                }
-            }
 
-         // send tls packets
-         if(m_core.hasDataToSend())
-            {
-            // \note: we construct `AsyncWriteOperation` with 0 as its last parameter (`plainBytesTransferred`).
-            //        This operation will eventually call `*this` as its own handler, passing the 0 back to this call
-            //        operator. This is necessary because, the check of `bytesTransferred > 0` assumes that
-            //        `bytesTransferred` bytes were just read and are in the cores input_buffer for further processing.
-            AsyncWriteOperation<
-            AsyncHandshakeOperation<typename std::decay<Handler>::type, Stream, Allocator>,
-                                    Stream,
-                                    Allocator>
-                                    op{std::move(*this), m_stream, m_core, 0};
-            boost::asio::async_write(m_stream.next_layer(), m_core.sendBuffer(), std::move(op));
-            return;
-            }
+            // we need more tls data from the socket
+            if(!m_stream.native_handle()->is_active() && !m_ec)
+               {
+               m_stream.next_layer().async_read_some(m_core.input_buffer, std::move(*this));
+               return;
+               }
 
-         // we need more tls data from the socket
-         if(!m_stream.native_handle()->is_active() && !ec)
-            {
-            m_stream.next_layer().async_read_some(m_core.input_buffer, std::move(*this));
-            return;
-            }
+            if(!isContinuation)
+               {
+               // this 0 byte read completes immediately. `yield` causes the coroutine to reenter the function after
+               // this read, enabling us to call the handler, while respecting asios guarantee that the handler will not
+               // be called without an intermediate initiating function
+               yield m_stream.next_layer().async_read_some(boost::asio::mutable_buffer(), std::move(*this));
+               }
 
-         this->invoke(isContinuation, ec);
+            this->invoke_now(m_ec);
+            }
          }
 
    private:
-      Stream&      m_stream;
-      StreamCore&  m_core;
+      Stream&     m_stream;
+      StreamCore& m_core;
+
+      boost::system::error_code m_ec;
    };
 
 }  // namespace TLS
 
 }  // namespace Botan
 
+#include <boost/asio/unyield.hpp>
+
 #endif
diff --git a/src/lib/tls/asio/asio_async_read_op.h b/src/lib/tls/asio/asio_async_read_op.h
index ed66df07d..5399981c0 100644
--- a/src/lib/tls/asio/asio_async_read_op.h
+++ b/src/lib/tls/asio/asio_async_read_op.h
@@ -14,6 +14,8 @@
 #include <botan/internal/asio_includes.h>
 #include <botan/internal/asio_stream_core.h>
 
+#include <boost/asio/yield.hpp>
+
 namespace Botan {
 
 namespace TLS {
@@ -32,6 +34,7 @@ struct AsyncReadOperation : public AsyncBase<Handler, typename Stream::executor_
          , m_stream(stream)
          , m_core(core)
          , m_buffers(buffers)
+         , m_decodedBytes(0)
          {
          }
 
@@ -42,46 +45,59 @@ struct AsyncReadOperation : public AsyncBase<Handler, typename Stream::executor_
 
       void operator()(boost::system::error_code ec, std::size_t bytes_transferred, bool isContinuation = true)
          {
-         std::size_t decodedBytes = 0;
-
-         if(bytes_transferred > 0 && !ec)
+         m_ec = ec;
+         reenter(this)
             {
-            boost::asio::const_buffer read_buffer{m_core.input_buffer.data(), bytes_transferred};
-            try
+
+            if(bytes_transferred > 0 && !ec)
                {
-               m_stream.native_handle()->received_data(static_cast<const uint8_t*>(read_buffer.data()),
-                                                       read_buffer.size());
+               boost::asio::const_buffer read_buffer{m_core.input_buffer.data(), bytes_transferred};
+               try
+                  {
+                  m_stream.native_handle()->received_data(static_cast<const uint8_t*>(read_buffer.data()),
+                                                          read_buffer.size());
+                  }
+               catch(const std::exception&)
+                  {
+                  ec = convertException();
+                  }
                }
-            catch(const std::exception&)
+
+            if(!m_core.hasReceivedData() && !ec)
                {
-               ec = convertException();
+               // we need more tls packets from the socket
+               m_stream.next_layer().async_read_some(m_core.input_buffer, std::move(*this));
+               return;
                }
-            }
 
-         if(!m_core.hasReceivedData() && !ec)
-            {
-            // we need more tls packets from the socket
-            m_stream.next_layer().async_read_some(m_core.input_buffer, std::move(*this));
-            return;
-            }
+            if(m_core.hasReceivedData() && !ec)
+               {
+               m_decodedBytes = m_core.copyReceivedData(m_buffers);
+               ec = {};
+               }
 
-         if(m_core.hasReceivedData() && !ec)
-            {
-            decodedBytes = m_core.copyReceivedData(m_buffers);
-            ec = {};
-            }
+            if(!isContinuation)
+               {
+               yield m_stream.next_layer().async_read_some(boost::asio::mutable_buffer(), std::move(*this));
+               }
 
-         this->invoke(isContinuation, ec, decodedBytes);
+            this->invoke_now(m_ec, m_decodedBytes);
+            }
          }
 
    private:
       Stream&               m_stream;
       StreamCore&           m_core;
       MutableBufferSequence m_buffers;
+
+      boost::system::error_code m_ec;
+      size_t                    m_decodedBytes;
    };
 
 }  // namespace TLS
 
 }  // namespace Botan
 
+#include <boost/asio/unyield.hpp>
+
 #endif
diff --git a/src/lib/tls/asio/asio_async_write_op.h b/src/lib/tls/asio/asio_async_write_op.h
index 67cc2cd5b..735d1b4eb 100644
--- a/src/lib/tls/asio/asio_async_write_op.h
+++ b/src/lib/tls/asio/asio_async_write_op.h
@@ -9,9 +9,11 @@
 #ifndef BOTAN_ASIO_ASYNC_WRITE_OP_H_
 #define BOTAN_ASIO_ASYNC_WRITE_OP_H_
 
-#include <botan/internal/asio_stream_core.h>
-#include <botan/internal/asio_includes.h>
 #include <botan/internal/asio_async_base.h>
+#include <botan/internal/asio_includes.h>
+#include <botan/internal/asio_stream_core.h>
+
+#include <boost/asio/yield.hpp>
 
 namespace Botan {
 
@@ -41,19 +43,33 @@ struct AsyncWriteOperation : public AsyncBase<Handler, typename Stream::executor
 
    void operator()(boost::system::error_code ec, std::size_t bytes_transferred, bool isContinuation = true)
       {
-      m_core.consumeSendBuffer(bytes_transferred);
-      // the size of the sent TLS record can differ from the size of the payload due to TLS encryption. We need to tell
-      // the handler how many bytes of the original data we already processed.
-      this->invoke(isContinuation, ec, ec ? 0 : m_plainBytesTransferred);
+      m_ec = ec;
+      reenter(this)
+         {
+         m_core.consumeSendBuffer(bytes_transferred);
+
+         if(!isContinuation)
+            {
+            yield m_stream.next_layer().async_write_some(boost::asio::const_buffer(), std::move(*this));
+            }
+
+         // the size of the sent TLS record can differ from the size of the payload due to TLS encryption. We need to tell
+         // the handler how many bytes of the original data we already processed.
+         this->invoke_now(m_ec, m_ec ? 0 : m_plainBytesTransferred);
+         }
       }
 
-   Stream&      m_stream;
-   StreamCore&  m_core;
+   Stream&     m_stream;
+   StreamCore& m_core;
    std::size_t m_plainBytesTransferred;
+
+   boost::system::error_code m_ec;
    };
 
 }  // namespace TLS
 
 }  // namespace Botan
 
+#include <boost/asio/unyield.hpp>
+
 #endif
diff --git a/src/lib/tls/asio/asio_error.h b/src/lib/tls/asio/asio_error.h
index e6bd01a6b..bbb52c435 100644
--- a/src/lib/tls/asio/asio_error.h
+++ b/src/lib/tls/asio/asio_error.h
@@ -44,12 +44,9 @@ enum class error
    unknown
    };
 
-using error_code = boost::system::error_code;
-using error_category = boost::system::error_category;
-
 namespace detail {
 // TLS Alerts
-struct BotanAlertCategory : error_category
+struct BotanAlertCategory : boost::system::error_category
    {
    const char* name() const noexcept override
       {
@@ -69,7 +66,7 @@ inline const BotanAlertCategory& botan_alert_category() noexcept
    return category;
    }
 
-struct BotanErrorCategory : error_category
+struct BotanErrorCategory : boost::system::error_category
    {
    const char* name() const noexcept override
       {
@@ -138,14 +135,14 @@ inline const BotanErrorCategory& botan_category() noexcept
    }
 } // namespace detail
 
-inline error_code make_error_code(Botan::TLS::Alert::Type c)
+inline boost::system::error_code make_error_code(Botan::TLS::Alert::Type c)
    {
-   return error_code(static_cast<int>(c), detail::botan_alert_category());
+   return boost::system::error_code(static_cast<int>(c), detail::botan_alert_category());
    }
 
-inline error_code make_error_code(error c)
+inline boost::system::error_code make_error_code(error c)
    {
-   return error_code(static_cast<int>(c), detail::botan_category());
+   return boost::system::error_code(static_cast<int>(c), detail::botan_category());
    }
 
 }  // namespace TLS