Split wait_manager arguments 23/274723/2
authorMateusz Majewski <m.majewski2@samsung.com>
Thu, 5 May 2022 09:34:49 +0000 (11:34 +0200)
committerMateusz Majewski <m.majewski2@samsung.com>
Mon, 9 May 2022 05:58:13 +0000 (07:58 +0200)
Change-Id: I03094609fab5072507081ee2ec4b437d3e602583

sessiond/src/main.cpp
sessiond/src/wait_manager.hpp

index d314c91..3debd27 100644 (file)
@@ -133,7 +133,7 @@ struct sessiond_context {
                fs_helpers::add_user_subsession(session_uid, subsession_id);
 
                wait_add.try_emplace(session_uid, session_uid, connection, "AddUserCompleted");
-               wait_add.at(session_uid).on_start(subsession_id);
+               wait_add.at(session_uid).on_start(subsession_id, { });
 
                g_dbus_method_invocation_return_value(invocation, nullptr);
        }
@@ -155,7 +155,7 @@ struct sessiond_context {
                fs_helpers::remove_user_subsession(session_uid, subsession_id);
 
                wait_remove.try_emplace(session_uid, session_uid, connection, "RemoveUserCompleted");
-               wait_remove.at(session_uid).on_start(subsession_id);
+               wait_remove.at(session_uid).on_start(subsession_id, { });
 
                g_dbus_method_invocation_return_value(invocation, nullptr);
        }
@@ -177,7 +177,7 @@ struct sessiond_context {
                        g_error_throw(err, "Failed to emit a signal: ");
 
                wait_switch.try_emplace(session_uid, session_uid, connection, "SwitchUserCompleted");
-               wait_switch.at(session_uid).on_start({switch_id, prev_subsession_id, next_subsession_id});
+               wait_switch.at(session_uid).on_start(switch_id, { prev_subsession_id, next_subsession_id });
 
                g_dbus_method_invocation_return_value(invocation, nullptr);
        }
@@ -267,17 +267,7 @@ struct sessiond_context {
                }
 
                wait_switch.try_emplace(session_uid, session_uid, connection, "SwitchUserCompleted");
-
-               // TODO: This is very, very bad. But the deadline is close...
-               auto waiting_data = wait_switch.at(session_uid).waiting;
-               auto full_data = std::find_if(waiting_data.begin(), waiting_data.end(), [&](auto pair) {
-                       return std::get<0>(pair.first) == switch_id;
-               });
-               if (full_data == waiting_data.end()) {
-                       g_dbus_method_invocation_return_dbus_error(invocation, "org.freedesktop.DBus.Error.InvalidArgs", "Nonexistant switch_id passed");
-                       return;
-               }
-               wait_switch.at(session_uid).on_client_done(full_data->first, std::string(sender));
+               wait_remove.at(session_uid).on_client_done(switch_id, std::string(sender));
 
                g_dbus_method_invocation_return_value(invocation, nullptr);
        }
@@ -430,9 +420,11 @@ struct sessiond_context {
                std::make_pair("GetUserList",    &sessiond_context::on_get_user_list),
        };
 
-       std::unordered_map<int, wait_manager<int>>      wait_add;
-       std::unordered_map<int, wait_manager<int>>      wait_remove;
-       std::unordered_map<int, wait_manager<uint64_t, int, int>> wait_switch;
+       // TODO: Currently, the first parameter is always a single-element tuple.
+       // Consider simplifying wait_manager.
+       std::unordered_map<int, wait_manager<std::tuple<int>, std::tuple<>>> wait_add;
+       std::unordered_map<int, wait_manager<std::tuple<int>, std::tuple<>>> wait_remove;
+       std::unordered_map<int, wait_manager<std::tuple<uint64_t>, std::tuple<int, int>>> wait_switch;
 
        uint64_t switch_id = 0;
 
index 80774dd..a81c56e 100644 (file)
@@ -38,7 +38,7 @@
 #include "tuple_g_variant_helpers.hpp"
 #include "tuple_hash.hpp"
 
-template<typename ...Vs>
+template<typename Primary, typename Secondary>
 struct wait_manager {
        wait_manager(int session_uid, GDBusConnection *connection, std::string_view completion_signal)
                        : session_uid(session_uid), connection(connection), completion_signal(completion_signal) {}
@@ -51,8 +51,8 @@ struct wait_manager {
                // (std::uncaught_exception exists, but there's no need for it IMHO.)
                try {
                        for (auto &waiting_for : waiting) {
-                               waiting_for.second.clear();
-                               finalize_if_empty(waiting_for.first);
+                               waiting_for.second.first.clear();
+                               finalize_if_empty(waiting_for.first, waiting_for.second.second);
                        }
                } catch (const std::exception &ex) {
                        std::cerr << "Exception " << ex.what() << "\n" <<
@@ -76,7 +76,7 @@ struct wait_manager {
                registered.emplace(name, std::move(match));
        }
 
-       void on_start(std::tuple<Vs...> v)
+       void on_start(Primary v, Secondary a)
        {
                // See the comment in on_timeout.
                timeouts.erase(std::remove_if(timeouts.begin(), timeouts.end(),
@@ -84,7 +84,7 @@ struct wait_manager {
                               timeouts.end());
 
                // See the comment in finalize.
-               std::erase_if(waiting, [](const auto &waits) { return waits.second.empty(); });
+               std::erase_if(waiting, [](const auto &waits) { return waits.second.first.empty(); });
 
                if (waiting.contains(v))
                        return; // TODO: any error here?
@@ -96,7 +96,7 @@ struct wait_manager {
                for (const auto &going_to_wait : registered)
                        waits.emplace(going_to_wait.first);
                if (waits.empty())
-                       signal(v);
+                       signal(v, a);
                else {
                        auto td = std::make_unique<timeout_data>();
                        td->self = this;
@@ -104,53 +104,55 @@ struct wait_manager {
                        td->active = true;
                        td->timeout_id = g_timeout_add_seconds(timeout.count(), glib_timeout, td.get());
 
-                       waiting.emplace(v, std::move(waits));
+                       waiting.emplace(v, std::make_pair(std::move(waits), a));
                        timeouts.emplace_back(std::move(td));
                }
        }
 
-       void on_client_done(std::tuple<Vs...> v, const std::string &name)
+       void on_client_done(Primary v, const std::string &name)
        {
                if (!waiting.contains(v))
                        return; // This could be an error, but then we would have a race condition with the timeout.
-               drop_name(v, waiting[v], name);
+               drop_name(v, waiting[v].first, waiting[v].second, name);
        }
 
        void on_client_disappeared(const std::string &name)
        {
                registered.erase(name);
                for (auto &waiting_for : waiting) {
-                       drop_name(waiting_for.first, waiting_for.second, name);
+                       drop_name(waiting_for.first, waiting_for.second.first, waiting_for.second.second, name);
                }
        }
 
-       void drop_name(std::tuple<Vs...> v, std::unordered_set<std::string> &waits, const std::string &name)
+       void drop_name(Primary v, std::unordered_set<std::string> &waits, Secondary aux, const std::string &name)
        {
                waits.erase(name);
-               finalize_if_empty(v);
+               finalize_if_empty(v, aux);
        }
 
-       void on_timeout(std::tuple<Vs...> v)
+       void on_timeout(Primary v)
        {
                if (!waiting.contains(v))
                        return;
-               waiting[v].clear();
 
-               finalize_if_empty(v);
+               auto &waited_v = waiting[v];
+               waited_v.first.clear();
+
+               finalize_if_empty(v, waited_v.second);
 
                // One thing that we'd like to do here is removing the timeout from the array.
                // However doing so is a bit sus, since glib_timeout still has pointer.
                // What we do instead, is clearing the array up a bit when adding a new timeout in on_start.
        }
 
-       void finalize_if_empty(std::tuple<Vs...> v)
+       void finalize_if_empty(Primary v, Secondary a)
        {
                // TODO: I don't like this, somehow.
                // Refactor suggestions are more than welcome.
-               if (!waiting.contains(v) && !waiting[v].empty())
+               if (!waiting.contains(v) && !waiting[v].first.empty())
                        return;
 
-               signal(v);
+               signal(v, a);
 
                // Again, we'd like to remove the now empty vector.
                // However, doing so is again sus, since other functions (for example on_client_disappeared)
@@ -158,11 +160,11 @@ struct wait_manager {
                // Again, let's clean up in on_start instead.
        }
 
-       void signal(std::tuple<Vs...> v)
+       void signal(Primary v, Secondary a)
        {
                GError *err = nullptr;
                if (!g_dbus_connection_emit_signal(connection, nullptr, bus_object.data(), bus_iface.data(), completion_signal.data(),
-                               tuple_to_g_variant(std::tuple_cat(std::make_tuple(session_uid), v)), &err))
+                               tuple_to_g_variant(std::tuple_cat(std::make_tuple(session_uid), v, a)), &err))
                        g_error_throw(err, "Failed to emit a signal: ");
        }
 
@@ -179,7 +181,7 @@ struct wait_manager {
        struct timeout_data {
                guint timeout_id;
                wait_manager *self;
-               std::tuple<Vs...> v;
+               Primary v;
                bool active;
        };
        static gboolean glib_timeout(gpointer user_data)
@@ -200,7 +202,14 @@ struct wait_manager {
        std::string_view completion_signal;
 
        std::unordered_map<std::string, guint> registered = {};
-       std::unordered_map<std::tuple<Vs...>, std::unordered_set<std::string>, tuple_hash<std::tuple<Vs...>>> waiting = {};
+       std::unordered_map<
+               Primary,
+               std::pair<
+                       std::unordered_set<std::string>,
+                       Secondary
+               >,
+               tuple_hash<Primary>
+       > waiting = {};
 
        // We hold all the timeout_data, because I couldn't get memory management to work otherwise.
        std::vector<std::unique_ptr<timeout_data>> timeouts = {};