refactor: rename ProxyClient<Thread>::m_cleanup_it

Rename `ProxyClient<Thread>` `m_cleanup_it` member to `m_disconnect_cb`.

There is no change in behavor in this commit. The change is just being made so
a bugfix in an upcoming commit will be more obvious.

The `m_cleanup_it` name was a bad name because it didn't make it clear that
this cleanup function only runs on sudden disconnects, not clean shutdowns. It
was also confusing because this is a member of ProxyClient object, which has
its own cleanup functions, but this doesn't actually refer to one of those
functions, it refers to a cleanup function in an entirely different object (the
Connection object).

Author: ryanofsky

include/mp/proxy-io.h

@@ -66,16 +66,16 @@ struct ProxyClient<Thread> : public ProxyClientBase<Thread, ::capnp::Void>
     ProxyClient(const ProxyClient&) = delete;
     ~ProxyClient();
 
-    void setCleanup(const std::function<void()>& fn);
-
-    //! Cleanup function to run when the connection is closed. If the Connection
-    //! gets destroyed before this ProxyClient<Thread> object, this cleanup
-    //! callback lets it destroy this object and remove its entry in the
-    //! thread's request_threads or callback_threads map (after resetting
-    //! m_cleanup_it so the destructor does not try to access it). But if this
-    //! object gets destroyed before the Connection, there's no need to run the
-    //! cleanup function and the destructor will unregister it.
-    std::optional<CleanupIt> m_cleanup_it;
+    void setDisconnectCallback(const std::function<void()>& fn);
+
+    //! Reference to callback function that is run if there is a sudden
+    //! disconnect and the Connection object is destroyed before this
+    //! ProxyClient<Thread> object. The callback will destroy this object and
+    //! remove its entry from the thread's request_threads or callback_threads
+    //! map. It will also reset m_disconnect_cb so the destructor does not
+    //! access it. In the normal case where there is no sudden disconnect, the
+    //! destructor will unregister m_disconnect_cb so the callback is never run.
+    std::optional<CleanupIt> m_disconnect_cb;
 };
 
 template <>

src/mp/proxy.cpp

@@ -311,16 +311,16 @@ std::tuple<ConnThread, bool> SetThread(ConnThreads& threads, std::mutex& mutex,
     thread = threads.emplace(
         std::piecewise_construct, std::forward_as_tuple(connection),
         std::forward_as_tuple(make_thread(), connection, /* destroy_connection= */ false)).first;
-    thread->second.setCleanup([&threads, &mutex, thread] {
+    thread->second.setDisconnectCallback([&threads, &mutex, thread] {
         // Note: it is safe to use the `thread` iterator in this cleanup
         // function, because the iterator would only be invalid if the map entry
         // was removed, and if the map entry is removed the ProxyClient<Thread>
         // destructor unregisters the cleanup.
 
         // Connection is being destroyed before thread client is, so reset
-        // thread client m_cleanup_it member so thread client destructor does not
+        // thread client m_disconnect_cb member so thread client destructor does not
         // try unregister this callback after connection is destroyed.
-        thread->second.m_cleanup_it.reset();
+        thread->second.m_disconnect_cb.reset();
         // Remove connection pointer about to be destroyed from the map
         const std::unique_lock<std::mutex> lock(mutex);
         threads.erase(thread);
@@ -333,16 +333,16 @@ ProxyClient<Thread>::~ProxyClient()
     // If thread is being destroyed before connection is destroyed, remove the
     // cleanup callback that was registered to handle the connection being
     // destroyed before the thread being destroyed.
-    if (m_cleanup_it) {
-        m_context.connection->removeSyncCleanup(*m_cleanup_it);
+    if (m_disconnect_cb) {
+        m_context.connection->removeSyncCleanup(*m_disconnect_cb);
     }
 }
 
-void ProxyClient<Thread>::setCleanup(const std::function<void()>& fn)
+void ProxyClient<Thread>::setDisconnectCallback(const std::function<void()>& fn)
 {
     assert(fn);
-    assert(!m_cleanup_it);
-    m_cleanup_it = m_context.connection->addSyncCleanup(fn);
+    assert(!m_disconnect_cb);
+    m_disconnect_cb = m_context.connection->addSyncCleanup(fn);
 }
 
 ProxyServer<Thread>::ProxyServer(ThreadContext& thread_context, std::thread&& thread)