Finally make DbusIpcThread terminate

...by directly calling shutdown(3) on the underlying socket, to make
dbus_connection_read_write fall out of its internal poll(3) call blocked on
POLLIN (upon which it returns false).  (dbus_connection_close only calls
close(3), so calling it from DbusIpcThread::close would merely decrement the
socket file descriptor's reference count.)  This removes the need for the sent-
to-self "Close" command (whose processing turned out to be too brittle in
parallel with closing the connection down, witness my previous three fruitless
commits in this area).

There appears to be no need to explicitly call dbus_bus_release_name, as the
session bus apparently takes care of releasing the name as soon as the
associated connection is closed.

Also there should be no need to call dbus_connection_read_write_dispatch instead
of just dbus_connection_read_write, and dbus_message_pop_message should probably
be called in a loop, until all queued messages are processed.

Change-Id: I13f30b6676a531f349e953329e910c1ff45ee53e

1 files changed, 61 insertions, 105 deletions

diff --git a/desktop/source/app/officeipcthread.cxx b/desktop/source/app/officeipcthread.cxx
index 8ad2602fade7..5b60248f6a7a 100644
--- a/desktop/source/app/officeipcthread.cxx
+++ b/desktop/source/app/officeipcthread.cxx
@@ -53,6 +53,7 @@
 
 #if ENABLE_DBUS
 #include <dbus/dbus.h>
+#include <sys/socket.h>
 #endif
 
 using namespace desktop;
@@ -449,7 +450,6 @@ private:
     void close() override;
 
     DbusConnectionHolder connection_;
-    osl::Condition closeDone_;
 };
 
 RequestHandler::Status DbusIpcThread::enable(rtl::Reference<IpcThread> * thread)
@@ -561,120 +561,76 @@ void DbusIpcThread::execute()
                 break;
             }
         }
-        dbus_connection_read_write_dispatch(connection_.connection, -1);
-        DbusMessageHolder msg(
-            dbus_connection_pop_message(connection_.connection));
-        if (msg.message == nullptr) {
-            continue;
-        }
-        if (dbus_message_is_method_call(
-                msg.message, "org.libreoffice.LibreOfficeIpcIfc0", "Close"))
-        {
+        if (!dbus_connection_read_write(connection_.connection, -1)) {
             break;
         }
-        if (!dbus_message_is_method_call(
-                msg.message, "org.libreoffice.LibreOfficeIpcIfc0", "Execute"))
-        {
-            SAL_INFO("desktop.app", "unknown DBus message ignored");
-            continue;
-        }
-        DBusMessageIter it;
-        if (!dbus_message_iter_init(msg.message, &it)) {
-            SAL_WARN("desktop.app", "DBus message without argument ignored");
-            continue;
-        }
-        if (dbus_message_iter_get_arg_type(&it) != DBUS_TYPE_STRING) {
-            SAL_WARN(
-                "desktop.app", "DBus message with non-string argument ignored");
-            continue;
-        }
-        char const * argstr;
-        dbus_message_iter_get_basic(&it, &argstr);
-        bool waitProcessed = false;
-        {
-            osl::MutexGuard g(RequestHandler::GetMutex());
-            if (!process(argstr, &waitProcessed)) {
+        for (;;) {
+            DbusMessageHolder msg(
+                dbus_connection_pop_message(connection_.connection));
+            if (msg.message == nullptr) {
+                break;
+            }
+            if (!dbus_message_is_method_call(
+                    msg.message, "org.libreoffice.LibreOfficeIpcIfc0",
+                    "Execute"))
+            {
+                SAL_INFO("desktop.app", "unknown DBus message ignored");
                 continue;
             }
+            DBusMessageIter it;
+            if (!dbus_message_iter_init(msg.message, &it)) {
+                SAL_WARN(
+                    "desktop.app", "DBus message without argument ignored");
+                continue;
+            }
+            if (dbus_message_iter_get_arg_type(&it) != DBUS_TYPE_STRING) {
+                SAL_WARN(
+                    "desktop.app",
+                    "DBus message with non-string argument ignored");
+                continue;
+            }
+            char const * argstr;
+            dbus_message_iter_get_basic(&it, &argstr);
+            bool waitProcessed = false;
+            {
+                osl::MutexGuard g(RequestHandler::GetMutex());
+                if (!process(argstr, &waitProcessed)) {
+                    continue;
+                }
+            }
+            if (waitProcessed) {
+                m_handler->cProcessed.wait();
+            }
+            DbusMessageHolder repl(dbus_message_new_method_return(msg.message));
+            if (repl.message == nullptr) {
+                SAL_WARN(
+                    "desktop.app", "dbus_message_new_method_return failed");
+                continue;
+            }
+            dbus_uint32_t serial = 0;
+            if (!dbus_connection_send(
+                    connection_.connection, repl.message, &serial)) {
+                SAL_WARN("desktop.app", "dbus_connection_send failed");
+                continue;
+            }
+            dbus_connection_flush(connection_.connection);
         }
-        if (waitProcessed) {
-            m_handler->cProcessed.wait();
-        }
-        DbusMessageHolder repl(dbus_message_new_method_return(msg.message));
-        if (repl.message == nullptr) {
-            SAL_WARN("desktop.app", "dbus_message_new_method_return failed");
-            continue;
-        }
-        dbus_uint32_t serial = 0;
-        if (!dbus_connection_send(
-                connection_.connection, repl.message, &serial)) {
-            SAL_WARN("desktop.app", "dbus_connection_send failed");
-            continue;
-        }
-        dbus_connection_flush(connection_.connection);
-    }
-    closeDone_.wait();
-    DBusError e;
-    dbus_error_init(&e);
-    int n = dbus_bus_release_name(
-        connection_.connection, "org.libreoffice.LibreOfficeIpc0", &e);
-    assert((n == -1) == bool(dbus_error_is_set(&e)));
-    switch (n) {
-    case -1:
-        SAL_WARN(
-            "desktop.app",
-            "dbus_bus_release_name failed with: " << e.name << ": "
-                << e.message);
-        dbus_error_free(&e);
-        break;
-    case DBUS_RELEASE_NAME_REPLY_RELEASED:
-        break;
-    case DBUS_RELEASE_NAME_REPLY_NOT_OWNER:
-    case DBUS_RELEASE_NAME_REPLY_NON_EXISTENT:
-        SAL_WARN(
-            "desktop.app",
-            "dbus_bus_release_name failed with unexpected " << +n);
-        break;
-    default:
-        for (;;) std::abort();
     }
 }
 
 void DbusIpcThread::close() {
-    {
-        assert(connection_.connection != nullptr);
-        DBusError e;
-        dbus_error_init(&e);
-        // Let DbusIpcThread::execute return from dbus_connection_read_write;
-        // for now, just abort on failure (the process would otherwise block,
-        // with DbusIpcThread::execute hanging in dbus_connection_read_write);
-        // this apparently needs a more DBus-y design anyway:
-        DbusConnectionHolder con(dbus_bus_get_private(DBUS_BUS_SESSION, &e));
-        assert((con.connection == nullptr) == bool(dbus_error_is_set(&e)));
-        if (con.connection == nullptr) {
-            SAL_WARN(
-                "desktop.app",
-                "dbus_bus_get_private failed with: " << e.name << ": "
-                    << e.message);
-            dbus_error_free(&e);
-            std::abort();
-        }
-        DbusMessageHolder msg(
-            dbus_message_new_method_call(
-                "org.libreoffice.LibreOfficeIpc0",
-                "/org/libreoffice/LibreOfficeIpc0",
-                "org.libreoffice.LibreOfficeIpcIfc0", "Close"));
-        if (msg.message == nullptr) {
-            SAL_WARN("desktop.app", "dbus_message_new_method_call failed");
-            std::abort();
-        }
-        if (!dbus_connection_send(con.connection, msg.message, nullptr)) {
-            SAL_WARN("desktop.app", "dbus_connection_send failed");
-            std::abort();
-        }
-        dbus_connection_flush(con.connection);
+    assert(connection_.connection != nullptr);
+    // Make dbus_connection_read_write fall out of internal poll call blocking
+    // on POLLIN:
+    int fd;
+    if (!dbus_connection_get_socket(connection_.connection, &fd)) {
+        SAL_WARN("desktop.app", "dbus_connection_get_socket failed");
+        return;
+    }
+    if (shutdown(fd, SHUT_RD) == -1) {
+        auto const e = errno;
+        SAL_WARN("desktop.app", "shutdown failed with errno " << e);
     }
-    closeDone_.set();
 }
 
 #endif