Use RAII for loop registration

There's probably a number of situations where registrations
with the loop are left behind and cause issues.  Instead, use
RAII to avoid this.

CMakeLists.txt

@@ -48,6 +48,7 @@ set(CommonSources
     include/verify_cache.h
     src/verify_cache.cpp
     include/i_loop.h
+    src/i_loop.cpp
     include/loop.h
     src/loop.cpp
     include/server.h

include/forwarder_connection.h

@@ -2,6 +2,7 @@
 #pragma once
 
 #include "config_parser.h"
+#include "i_loop.h"
 #include "openssl/ssl_connection.h"
 
 #include <memory>
@@ -12,7 +13,6 @@
 
 namespace dote {
 
-class ILoop;
 class IForwarderConfig;
 class Socket;
 
@@ -93,8 +93,6 @@ class ForwarderConnection
     void configureVerifier();
 
     /// \brief  Perform the initial connection
-    ///
-    /// \param handle  The socket that is available to connect on
     void connect(int handle);
 
     /// \brief  Nicely shutdown the connection
@@ -136,6 +134,12 @@ class ForwarderConnection
     State m_state;
     /// The established connection to the forwarder
     std::shared_ptr<Socket> m_socket;
+    /// The current read registration for m_socket.
+    ILoop::Registration m_read;
+    /// The current write registration for m_socket.
+    ILoop::Registration m_write;
+    /// The current exception registration for m_socket.
+    ILoop::Registration m_exception;
     /// The write buffer, the request will be a single message
     std::vector<char> m_buffer;
     /// The chosen forwarder that this is connected to

include/i_loop.h

@@ -12,6 +12,72 @@ class ILoop
   public:
     virtual ~ILoop() = default;
 
+    /// \brief  A registration type.
+    enum Type {
+      /// A special value to avoid deregistering on move.
+      Moved,
+      /// A read registration.
+      Read,
+      /// A write registration.
+      Write,
+      /// An exception registration.
+      Exception,
+    };
+
+    /// \brief  This class is created with a read, write or exception
+    ///         registration and automatically removes the registration
+    ///         on destruction.
+    ///
+    /// NOTE: This object must not outlive the Loop that created it.
+    class Registration
+    {
+      public:
+        Registration(const Registration&) = delete;
+        Registration& operator=(const Registration&) = delete;
+
+        /// \brief  Construct an invalid Registration.
+        Registration();
+
+        /// \brief  Create a registration for a given loop.
+        ///
+        /// \param loop    The loop that this registration is for.
+        /// \param handle  The handle of the registration
+        /// \param type    The type of the registration.
+        Registration(ILoop* loop, int handle, Type type);
+
+        /// \brief  Take ownership of a registration.
+        ///
+        /// \param other  The instance to take ownership from.
+        Registration(Registration&& other);
+
+        /// \brief  Take ownership of a registration.
+        ///
+        /// \param other  The instance to take ownership from.
+        Registration& operator=(Registration&& other);
+
+        /// \brief  Deregister from the loop.
+        ~Registration();
+
+        operator bool() const { return valid(); }
+
+        /// \brief  Whether the registration is valid (i.e. the execution was
+        //          successful).
+        bool valid() const;
+
+        /// \brief  Clear the registration.
+        void reset();
+
+      private:
+        friend class ILoop;
+
+        /// The loop that this registration is for.
+        ILoop* m_loop;
+        /// The handle to deregister.
+        int m_handle;
+        /// The type of registration.
+        Type m_type;
+    };
+
     /// The type to call when the loop is available, called with the
     /// handle that caused it to be called
     using Callback = std::function<void(int)>;
@@ -23,7 +89,7 @@ class ILoop
     /// \param timeout  The time at which to call exception on the handle
     ///
     /// \return  True if the handle is not already registered and now is
-    virtual bool registerRead(int handle, Callback callback, time_t timeout) = 0;
+    virtual Registration registerRead(int handle, Callback callback, time_t timeout) = 0;
 
     /// \brief   Register for write availability on a given handle
     ///
@@ -32,16 +98,17 @@ class ILoop
     /// \param timeout  The time at which to call exception on the handle
     ///
     /// \return  True if the handle is not already registered and now is
-    virtual bool registerWrite(int handle, Callback callback, time_t timeout) = 0;
+    virtual Registration registerWrite(int handle, Callback callback, time_t timeout) = 0;
 
     /// \brief  Register for exceptions on a given handle
     ///
     /// \param handle    The handle to register for exceptions
     /// \param callback  The callback to call if it triggers
     ///
     /// \return  True if the handle is not already registered and now is
-    virtual bool registerException(int handle, Callback callback) = 0;
+    virtual Registration registerException(int handle, Callback callback) = 0;
 
+  protected:
     /// \brief  Remove a read handle from the loop
     ///
     /// \param handle  The handle to remove read handles for

include/ip_lookup.h

@@ -1,6 +1,8 @@
 
 #pragma once
 
+#include "i_loop.h"
+
 #include <memory>
 
 namespace dote {
@@ -50,6 +52,12 @@ class IpLookup
     std::shared_ptr<Loop> m_loop;
     /// The socket that the connection if performed on
     std::shared_ptr<Socket> m_socket;
+    /// The current read registration for m_socket.
+    ILoop::Registration m_read;
+    /// The current write registration for m_socket.
+    ILoop::Registration m_write;
+    /// The current exception registration for m_socket.
+    ILoop::Registration m_exception;
     /// The connection that was made to the IP address
     std::shared_ptr<openssl::ISslConnection> m_connection;
     /// The time at which the lookup will be considered failed

include/loop.h

@@ -32,26 +32,27 @@ class Loop : public ILoop
     /// \param callback  The callback to call if it triggers
     /// \param timeout  The time at which to call exception on the handle
     ///
-    /// \return  True if the handle is not already registered and now is
-    bool registerRead(int handle, Callback callback, time_t timeout) override;
+    /// \return  A registration which is valid on success
+    Registration registerRead(int handle, Callback callback, time_t timeout) override;
 
     /// \brief   Register for write availability on a given handle
     ///
     /// \param handle    The handle to register for writing
     /// \param callback  The callback to call if it triggers
     /// \param timeout  The time at which to call exception on the handle
     ///
-    /// \return  True if the handle is not already registered and now is
-    bool registerWrite(int handle, Callback callback, time_t timeout) override;
+    /// \return  A registration which is valid on success
+    Registration registerWrite(int handle, Callback callback, time_t timeout) override;
 
     /// \brief  Register for exceptions on a given handle
     ///
     /// \param handle    The handle to register for exceptions
     /// \param callback  The callback to call if it triggers
     ///
-    /// \return  True if the handle is not already registered and now is
-    bool registerException(int handle, Callback callback) override;
+    /// \return  A registration which is valid on success
+    Registration registerException(int handle, Callback callback) override;
 
+  private:
     /// \brief  Remove a read handle from the loop
     ///
     /// \param handle  The handle to remove read handles for
@@ -67,7 +68,6 @@ class Loop : public ILoop
     /// \param handle  The handle to remove exception handles for
     void removeException(int handle) override;
 
-  private:
     /// \brief  Call a callback in a set of functions
     ///
     /// \param functions  The functions to lookup the callback in

include/server.h

@@ -2,13 +2,13 @@
 #pragma once
 
 #include "config_parser.h"
+#include "i_loop.h"
 
 #include <vector>
 #include <memory>
 
 namespace dote {
 
-class ILoop;
 class Socket;
 class IForwarders;
 
@@ -46,8 +46,9 @@ class Server
     std::shared_ptr<ILoop> m_loop;
     /// The available forwarders
     std::shared_ptr<IForwarders> m_forwarders;
-    /// The sockets that we are recieving from
-    std::vector<std::shared_ptr<Socket>> m_serverSockets;
+    using SocketAndRegistration = std::pair<std::shared_ptr<Socket>, ILoop::Registration>;
+    /// The sockets that we are recieving from and their read registrations.
+    std::vector<SocketAndRegistration> m_serverSockets;
 };
 
 }  // namespace dote

include/vyatta_check.h

@@ -2,6 +2,7 @@
 #pragma once
 
 #include "config_parser.h"
+#include "i_loop.h"
 
 namespace dote {
 
@@ -41,6 +42,8 @@ class VyattaCheck
 
     /// A handle to the inotify watch on the configuration file
     int m_fd;
+    /// The read registration for m_fd.
+    ILoop::Registration m_read;
     /// The main DoTe instance to configure on configuration changes
     Dote* m_dote;
     /// The base configuration to augment

src/forwarder_config.cpp

@@ -50,7 +50,7 @@ std::vector<ConfigParser::Forwarder>::const_iterator ForwarderConfig::end() cons
     return m_forwarders.cend();
 }
 
-void ForwarderConfig::setBad(const ConfigParser::Forwarder &config)
+void ForwarderConfig::setBad(const ConfigParser::Forwarder& config)
 {
     // Look for the config in the forwarder list and move it to the end
     for (auto it = m_forwarders.begin(); it != m_forwarders.end(); ++it)