From: Marcin Siodelski <marcin@isc.org>
Date: Mon, 17 Jun 2019 14:40:18 +0000 (+0200)
Subject: [#491,!363] Addressed review comments.
X-Git-Tag: Kea-1.6.0-beta2~283
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8b0b79ac56722164dc758ecdff827410cead9598;p=thirdparty%2Fkea.git

[#491,!363] Addressed review comments.

- Fixed one doxygen error in HttpListenerImpl
- Described lifecycle of the transaction within the Http connection.
- Use another variable in HttpConnection::doRead to be less confusing
- Fixed a typo in Http connection.
---

diff --git a/src/lib/http/connection.cc b/src/lib/http/connection.cc
index 331106b49e..d3d117a049 100644
--- a/src/lib/http/connection.cc
+++ b/src/lib/http/connection.cc
@@ -122,7 +122,7 @@ HttpConnection::doRead(TransactionPtr transaction) {
     try {
         TCPEndpoint endpoint;
 
-        // Transaction is was not created if we are starting to read the
+        // Transaction hasn't been created if we are starting to read the
         // new request.
         if (!transaction) {
             transaction = Transaction::create(response_creator_);
@@ -380,11 +380,11 @@ HttpConnection::requestTimeoutCallback(TransactionPtr transaction) {
     // We need to differentiate the transactions between a normal response and the
     // timeout. We create new transaction from the current transaction. It is
     // to preserve the request we're responding to.
-    transaction = Transaction::spawn(response_creator_, transaction);
+    auto spawned_transaction = Transaction::spawn(response_creator_, transaction);
 
     // The new transaction inherits the request from the original transaction
     // if such transaction exists.
-    auto request = transaction->getRequest();
+    auto request = spawned_transaction->getRequest();
 
     // Depending on when the timeout occured, the HTTP version of the request
     // may or may not be available. Therefore we check if the HTTP version is
@@ -404,7 +404,7 @@ HttpConnection::requestTimeoutCallback(TransactionPtr transaction) {
                                                    HttpStatusCode::REQUEST_TIMEOUT);
 
     // Send the HTTP 408 status.
-    asyncSendResponse(response, transaction);
+    asyncSendResponse(response, spawned_transaction);
 }
 
 void
diff --git a/src/lib/http/connection.h b/src/lib/http/connection.h
index c7ec7224d1..3c3c3ffad8 100644
--- a/src/lib/http/connection.h
+++ b/src/lib/http/connection.h
@@ -96,9 +96,25 @@ protected:
     /// parser (being in the particular state of parsing), input buffer
     /// and the output buffer.
     ///
-    /// A pointer to the @c Transaction object is passed to the ASIO
-    /// callbacks when the new message exchange begins. It is passed
-    /// between the callbacks until the message exchange is completed.
+    /// The new @c Transaction instance is created when the connection
+    /// is established and the server starts receiving the HTTP request.
+    /// The shared pointer to the created transaction is passed between
+    /// the asynchronous handlers. Therefore, as long as the asynchronous
+    /// communication is conducted the instance of the transaction is
+    /// held by the IO service which runs the handlers. The transaction
+    /// instance exists as long as the asynchronous handlers for the
+    /// given request/response exchange are executed. When the server
+    /// responds to the client and all corresponding IO handlers are
+    /// invoked the transaction is automatically destroyed.
+    ///
+    /// The timeout may occur anytime during the transaction. In such
+    /// cases, a new transaction instance is created to send the
+    /// HTTP 408 (timeout) response to the client. Creation of the
+    /// new transaction for the timeout response is necessary because
+    /// there may be some asynchronous handlers scheduled by the
+    /// original transaction which rely on the original transaction's
+    /// state. The timeout response's state is held within the new
+    /// transaction spawned from the original transaction.
     class Transaction {
     public:
 
diff --git a/src/lib/http/listener_impl.h b/src/lib/http/listener_impl.h
index c6a8952d9a..83e084cff4 100644
--- a/src/lib/http/listener_impl.h
+++ b/src/lib/http/listener_impl.h
@@ -81,7 +81,7 @@ protected:
 
     /// @brief Callback invoked when the new connection is accepted.
     ///
-    /// It calls @ref HttpListener::accept to create new @ref HttpConnection
+    /// It calls @c HttpListener::accept to create new @c HttpConnection
     /// instance.
     ///
     /// @param ec Error code passed to the handler. This is currently ignored.