[Libreoffice-commits] core.git: desktop/source

[Stephan Bergmann]

 desktop/source/app/officeipcthread.cxx |   84 +++++++++++++++++++++------------
 1 file changed, 54 insertions(+), 30 deletions(-)

New commits:
commit dc3ff192f3f048059cf149f416d0b237eb33b014
Author: Stephan Bergmann <sbergman at redhat.com>
Date:   Wed Oct 25 17:09:17 2017 +0200

    Avoid further unwanted interference of DbusIpcThread::execute/close
    
    ...after 38081c0884b64ed1132047973b4dccc42d548c89 "Avoid race between
    DbusIpcThread::close and DbusIpcThread::execute"
    
    Change-Id: I812f53525f4c2c051781321dac7096e3bf0d7054

diff --git a/desktop/source/app/officeipcthread.cxx b/desktop/source/app/officeipcthread.cxx
index b2380c0f9c43..b14cde421d24 100644
--- a/desktop/source/app/officeipcthread.cxx
+++ b/desktop/source/app/officeipcthread.cxx
@@ -449,6 +449,7 @@ private:
     void close() override;
 
     DbusConnectionHolder connection_;
+    osl::Condition closeDone_;
 };
 
 RequestHandler::Status DbusIpcThread::enable(rtl::Reference<IpcThread> * thread)
@@ -569,6 +570,19 @@ void DbusIpcThread::execute()
         if (dbus_message_is_method_call(
                 msg.message, "org.libreoffice.LibreOfficeIpcIfc0", "Close"))
         {
+            DbusMessageHolder repl(dbus_message_new_method_return(msg.message));
+            if (repl.message == nullptr) {
+                SAL_WARN(
+                    "desktop.app", "dbus_message_new_method_return failed");
+            } else {
+                dbus_uint32_t serial = 0;
+                if (!dbus_connection_send(
+                        connection_.connection, repl.message, &serial)) {
+                    SAL_WARN("desktop.app", "dbus_connection_send failed");
+                } else {
+                    dbus_connection_flush(connection_.connection);
+                }
+            }
             break;
         }
         if (!dbus_message_is_method_call(
@@ -612,6 +626,7 @@ void DbusIpcThread::execute()
         }
         dbus_connection_flush(connection_.connection);
     }
+    closeDone_.wait();
     DBusError e;
     dbus_error_init(&e);
     int n = dbus_bus_release_name(
@@ -639,37 +654,46 @@ void DbusIpcThread::execute()
 }
 
 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();
+    {
+        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();
+        }
+        DbusMessageHolder repl(
+            dbus_connection_send_with_reply_and_block(
+                con.connection, msg.message, 0x7FFFFFFF, &e));
+        assert((repl.message == nullptr) == bool(dbus_error_is_set(&e)));
+        if (repl.message == nullptr) {
+            SAL_INFO(
+                "desktop.app",
+                "dbus_connection_send_with_reply_and_block failed with: "
+                    << e.name << ": " << e.message);
+            dbus_error_free(&e);
+        }
     }
-    dbus_connection_flush(con.connection);
+    closeDone_.set();
 }
 
 #endif