- work on dbus interface

diff --git a/dbus/DBusConnection.cc b/dbus/DBusConnection.cc
index 49ffcba2b84796bc111c2925239e03a8c5eb1752..c7fd2f46188f843891976e20e22a164408187723 100644 (file)
--- a/dbus/DBusConnection.cc
+++ b/dbus/DBusConnection.cc
@@ -126,32 +126,11 @@ namespace DBus
     }
 
 
-    void
-    Connection::register_object_path(const char* path, const DBusObjectPathVTable* vtable,
-                                    void* user_data)
-    {
-       boost::lock_guard<boost::mutex> lock(mutex);
-
-       if (!dbus_connection_register_object_path(conn, path, vtable, user_data))
-           throw FatalException();
-    }
-
-
-    DBusDispatchStatus
-    Connection::get_dispatch_status()
+    DBusMessage*
+    Connection::pop_message()
     {
        boost::lock_guard<boost::mutex> lock(mutex);
-
-       return dbus_connection_get_dispatch_status(conn);
-    }
-
-
-    DBusDispatchStatus
-    Connection::dispatch()
-    {
-       boost::lock_guard<boost::mutex> lock(mutex);
-
-       return dbus_connection_dispatch(conn);
+       return dbus_connection_pop_message(conn);
     }
 
 

diff --git a/dbus/DBusConnection.h b/dbus/DBusConnection.h
index a3a318767b977998eaefee57245a67e9cd22a8b6..ebec42d6739957a4f0e83bd43419b08671fc65b5 100644 (file)
--- a/dbus/DBusConnection.h
+++ b/dbus/DBusConnection.h
@@ -42,23 +42,14 @@ namespace DBus
        Connection(DBusBusType type);
        ~Connection();
 
-       DBusConnection* get_connection() { return conn; }
-
        void request_name(const char* name, unsigned int flags);
 
        void send(Message& m);
 
        Message send_with_reply_and_block(Message& m);
 
-       DBusDispatchStatus get_dispatch_status();
-
-       DBusDispatchStatus dispatch();
-
        void add_match(const char* rule);
 
-       void register_object_path(const char* path, const DBusObjectPathVTable* vtable,
-                                 void* user_data);
-
        unsigned long get_unix_userid(const Message& m);
 
     protected:
@@ -70,6 +61,8 @@ namespace DBus
 
        DBusConnection* conn;
 
+       DBusMessage* pop_message();
+
     };
 
 }

diff --git a/dbus/DBusMainLoop.cc b/dbus/DBusMainLoop.cc
index 86c521a5a5ba9f3282dfccaea4156f437dcd74c5..9fcde768cd07f8ac2593586404df1f65e0fa110a 100644 (file)
--- a/dbus/DBusMainLoop.cc
+++ b/dbus/DBusMainLoop.cc
@@ -45,6 +45,8 @@ namespace DBus
            throw FatalException();
 
        dbus_connection_set_wakeup_main_function(conn, wakeup_main, this, NULL);
+
+       add_match("type='signal', interface='" DBUS_INTERFACE_DBUS "', member='NameOwnerChanged'");
     }
 
 
@@ -84,7 +86,7 @@ namespace DBus
                }
            }
 
-           int timeout = -1;
+           int timeout = periodic_timeout();
 
            if (idle_timeout >= 0)
            {
@@ -100,6 +102,8 @@ namespace DBus
            if (r == -1)
                throw FatalException();
 
+           periodic();
+
            {
                for (vector<struct pollfd>::const_iterator it2 = pollfds.begin(); it2 != pollfds.end(); ++it2)
                {
@@ -137,9 +141,11 @@ namespace DBus
                }
            }
 
-           while (get_dispatch_status() == DBUS_DISPATCH_DATA_REMAINS)
+           DBusMessage* tmp = pop_message();
+           if (tmp)
            {
-               dispatch();
+               DBus::Message msg(tmp, false);
+               dispatch_incoming(msg);
            }
 
            if (idle_timeout >= 0)
@@ -276,4 +282,38 @@ namespace DBus
        write(s->wakeup_pipe[1], &arbitrary, 1);
     }
 
+
+    void
+    DBus::MainLoop::dispatch_incoming(Message& msg)
+    {
+       switch (msg.get_type())
+       {
+           case DBUS_MESSAGE_TYPE_METHOD_CALL:
+           {
+               method_call(msg);
+           }
+           break;
+
+           case DBUS_MESSAGE_TYPE_SIGNAL:
+           {
+               signal(msg);
+
+               if (msg.is_signal(DBUS_INTERFACE_DBUS, "NameOwnerChanged"))
+               {
+                   string name, old_owner, new_owner;
+
+                   DBus::Hihi hihi(msg);
+                   hihi >> name >> old_owner >> new_owner;
+
+                   if (name == new_owner && old_owner.empty())
+                       client_connected(name);
+
+                   if (name == old_owner && new_owner.empty())
+                       client_disconnected(name);
+               }
+           }
+           break;
+       }
+    }
+
 }

diff --git a/dbus/DBusMainLoop.h b/dbus/DBusMainLoop.h
index 428c063ecde1ae8073c7e83c1c3481aec919dd44..5254576794d97ca0d7d552e3d7f2cd2ada40753e 100644 (file)
--- a/dbus/DBusMainLoop.h
+++ b/dbus/DBusMainLoop.h
@@ -37,13 +37,20 @@ namespace DBus
     public:
 
        MainLoop(DBusBusType type);
-       ~MainLoop();
+       virtual ~MainLoop();
 
        void run();
 
        void set_idle_timeout(int s);
        void reset_idle_count();
 
+       virtual void method_call(Message& message) = 0;
+       virtual void signal(Message& message) = 0;
+       virtual void client_connected(const string& name) = 0;
+       virtual void client_disconnected(const string& name) = 0;
+       virtual int periodic_timeout() = 0;
+       virtual void periodic() = 0;
+
     protected:
 
        struct Watch
@@ -79,6 +86,8 @@ namespace DBus
 
        static void wakeup_main(void* data);
 
+       void dispatch_incoming(Message& message);
+
        int idle_timeout;
        time_t last_action;
 

diff --git a/server/Client.cc b/server/Client.cc
index e3f518fcf49a5f8fb4a740f6165d9d899a67eb30..523110a996f3ae034f98873470aa2095f2b91ffb 100644 (file)
--- a/server/Client.cc
+++ b/server/Client.cc
@@ -32,7 +32,7 @@ bool contains(const ListType& l, const Type& value)
 
 
 Client::Client(const string& name)
-    : name(name)
+    : name(name), zombie(false)
 {
 }
 
@@ -41,7 +41,7 @@ Client::~Client()
 {
     thread.interrupt();
 
-    thread.join();             // TODO this can block and deadlock
+    thread.join();
 
     for (list<Comparison*>::iterator it = comparisons.begin(); it != comparisons.end(); ++it)
     {
@@ -168,9 +168,24 @@ Clients::add(const string& name)
 
 
 void
-Clients::remove(const string& name)
+Clients::remove_zombies()
 {
-    iterator it = find(name);
-    if (it != entries.end())
-       entries.erase(it);
+    for (iterator it = begin(); it != end();)
+    {
+       if (it->zombie && it->thread.timed_join(boost::posix_time::seconds(0)))
+           it = entries.erase(it);
+       else
+           ++it;
+    }
+}
+
+
+bool
+Clients::has_zombies() const
+{
+    for (const_iterator it = begin(); it != end(); ++it)
+       if (it->zombie)
+           return true;
+
+    return false;
 }

diff --git a/server/Client.h b/server/Client.h
index 47a6f6b1c2f3e6cd2ef87b1e5f2a96357505f6b4..a41413171d9fe91da55fa517a91f5d2cbb825a2b 100644 (file)
--- a/server/Client.h
+++ b/server/Client.h
@@ -119,6 +119,8 @@ public:
     boost::thread thread;
     queue<Task> tasks;
 
+    bool zombie;
+
     void add_task(DBus::Connection& conn, DBus::Message& msg);
 
     void worker();
@@ -144,7 +146,9 @@ public:
     iterator find(const string& name);
 
     iterator add(const string& name);
-    void remove(const string& name);
+    void remove_zombies();
+
+    bool has_zombies() const;
 
 private:
 

diff --git a/server/snapperd.cc b/server/snapperd.cc
index f1ebadadb4aa67eb7017a309033427a0aa47e07f..a2f10f5ff8703bad1a2f197ac924f6c9f2c1fca0 100644 (file)
--- a/server/snapperd.cc
+++ b/server/snapperd.cc
@@ -1085,6 +1085,8 @@ Client::debug(DBus::Connection& conn, DBus::Message& msg)
        s << "    name:'" << it->name << "'";
        if (&*it == this)
            s << " myself";
+       if (it->zombie)
+           s << " zombie";
        if (!it->locks.empty())
            s << " locks:'" << boost::join(it->locks, ",") << "'";
        if (!it->comparisons.empty())
@@ -1108,20 +1110,6 @@ Client::debug(DBus::Connection& conn, DBus::Message& msg)
 }
 
 
-Clients::iterator
-client_connected(const string& name)
-{
-    return clients.add(name);
-}
-
-
-void
-client_disconnected(const string& name)
-{
-    clients.remove(name);
-}
-
-
 void
 Client::dispatch(DBus::Connection& conn, DBus::Message& msg)
 {
@@ -1227,103 +1215,131 @@ Client::dispatch(DBus::Connection& conn, DBus::Message& msg)
 }
 
 
-void
-unregister_func(DBusConnection* connection, void* data)
+class MyMainLoop : public DBus::MainLoop
+{
+public:
+
+    MyMainLoop(DBusBusType type);
+    ~MyMainLoop();
+
+    void method_call(DBus::Message& message);
+    void signal(DBus::Message& message);
+    void client_connected(const string& name);
+    void client_disconnected(const string& name);
+    void periodic();
+    int periodic_timeout();
+
+};
+
+
+MyMainLoop::MyMainLoop(DBusBusType type)
+    : MainLoop(type)
 {
 }
 
 
-DBusHandlerResult
-message_func1(DBusConnection* connection, DBusMessage* message, void* data)
+MyMainLoop::~MyMainLoop()
 {
-    DBus::MainLoop* s = static_cast<DBus::MainLoop*>(data);
+}
 
-    DBus::Message msg(message, true);
 
-    if (msg.get_type() == DBUS_MESSAGE_TYPE_METHOD_CALL)
-    {
-       y2deb("method call sender:'" << msg.get_sender() << "' path:'" <<
-             msg.get_path() << "' interface:'" << msg.get_interface() <<
-             "' member:'" << msg.get_member() << "'");
+void
+MyMainLoop::method_call(DBus::Message& msg)
+{
+    y2deb("method call sender:'" << msg.get_sender() << "' path:'" <<
+         msg.get_path() << "' interface:'" << msg.get_interface() <<
+         "' member:'" << msg.get_member() << "'");
 
-       s->reset_idle_count();
+    reset_idle_count();
 
-       if (msg.is_method_call(DBUS_INTERFACE_INTROSPECTABLE, "Introspect"))
-       {
-           Client::introspect(*s, msg);
-       }
-       else
+    if (msg.is_method_call(DBUS_INTERFACE_INTROSPECTABLE, "Introspect"))
+    {
+       Client::introspect(*this, msg);
+    }
+    else
+    {
+       boost::unique_lock<boost::shared_mutex> lock(big_mutex);
+
+       Clients::iterator client = clients.find(msg.get_sender());
+       if (client == clients.end())
        {
-           Clients::iterator client = clients.find(msg.get_sender());
-           if (client == clients.end())
-           {
-               y2deb("client connected invisible '" << msg.get_sender() << "'");
-               client = client_connected(msg.get_sender());
-               s->set_idle_timeout(-1);
-           }
-
-           client->add_task(*s, msg);
+           y2deb("client connected invisible '" << msg.get_sender() << "'");
+           client = clients.add(msg.get_sender());
+           set_idle_timeout(-1);
        }
 
-       return DBUS_HANDLER_RESULT_HANDLED;
+       client->add_task(*this, msg);
     }
+}
+
+
+void
+MyMainLoop::signal(DBus::Message& msg)
+{
+    y2deb("signal sender:'" << msg.get_sender() << "' path:'" <<
+         msg.get_path() << "' interface:'" << msg.get_interface() <<
+         "' member:'" << msg.get_member() << "'");
+}
+
+
+void
+MyMainLoop::client_connected(const string& name)
+{
+    y2deb("client connected '" << name << "'");
+
+    boost::unique_lock<boost::shared_mutex> lock(big_mutex);
 
-    return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+    clients.add(name);
+
+    reset_idle_count();
+    set_idle_timeout(-1);
 }
 
 
-DBusHandlerResult
-message_func2(DBusConnection* connection, DBusMessage* message, void* data)
+void
+MyMainLoop::client_disconnected(const string& name)
 {
-    DBus::MainLoop* s = static_cast<DBus::MainLoop*>(data);
+    y2deb("client disconnected '" << name << "'");
 
-    DBus::Message msg(message, true);
+    boost::unique_lock<boost::shared_mutex> lock(big_mutex);
 
-    if (msg.get_type() == DBUS_MESSAGE_TYPE_SIGNAL)
+    Clients::iterator client = clients.find(name);
+    if (client != clients.end())
     {
-       y2deb("signal sender:'" << msg.get_sender() << "' path:'" <<
-             msg.get_path() << "' interface:'" << msg.get_interface() <<
-             "' member:'" << msg.get_member() << "'");
-
-       if (msg.is_signal(DBUS_INTERFACE_DBUS, "NameOwnerChanged"))
-       {
-           string name, old_owner, new_owner;
-
-           DBus::Hihi hihi(msg);
-           hihi >> name >> old_owner >> new_owner;
-
-           if (name == new_owner && old_owner.empty())
-           {
-               y2deb("client connected '" << name << "'");
-               client_connected(name);
-               s->reset_idle_count();
-               s->set_idle_timeout(-1);
-           }
-
-           if (name == old_owner && new_owner.empty())
-           {
-               y2deb("client disconnected '" << name << "'");
-               client_disconnected(name);
-               s->reset_idle_count();
-               if (clients.empty())
-                   s->set_idle_timeout(30);
-           }
-
-           return DBUS_HANDLER_RESULT_HANDLED;
-       }
+       client->zombie = true;
+       client->thread.interrupt();
     }
+    reset_idle_count();
+}
+
 
-    return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+void
+MyMainLoop::periodic()
+{
+    y2deb("periodic");
+
+    boost::unique_lock<boost::shared_mutex> lock(big_mutex);
+
+    clients.remove_zombies();
+
+    if (clients.empty())
+    {
+       y2deb("no clients left");
+       set_idle_timeout(30);
+    }
 }
 
 
-DBusObjectPathVTable dbus_vtable1 = {
-    unregister_func, message_func1, NULL, NULL, NULL, NULL
-};
+int
+MyMainLoop::periodic_timeout()
+{
+    boost::unique_lock<boost::shared_mutex> lock(big_mutex);
 
-DBusObjectPathVTable dbus_vtable2 = {
-    unregister_func, message_func2, NULL, NULL, NULL, NULL
-};
+    if (clients.has_zombies())
+       return 1000;
+
+    return -1;
+}
 
 
 void
@@ -1358,7 +1374,7 @@ main(int argc, char** argv)
 
     dbus_threads_init_default();
 
-    DBus::MainLoop mainloop(DBUS_BUS_SYSTEM);
+    MyMainLoop mainloop(DBUS_BUS_SYSTEM);
 
     mainloop.set_idle_timeout(30);
 
@@ -1368,10 +1384,6 @@ main(int argc, char** argv)
 
     y2mil("Listening for method calls and signals");
 
-    mainloop.register_object_path(PATH, &dbus_vtable1, &mainloop);
-    mainloop.register_object_path(DBUS_PATH_DBUS, &dbus_vtable2, &mainloop);
-    mainloop.add_match("type='signal', interface='" DBUS_INTERFACE_DBUS "', member='NameOwnerChanged'");
-
     mainloop.run();
 
     y2mil("Exiting");