Issue #244: Fixed issue with shared connections failing when dbus dae…

…mon disconnected before

This issue is triggered when shared connections are enabled (default)
and you create a connection to a DBus daemon, then the daemon is
restarted (process killed and restarted) causing your connection to be
closed. When retrying to connect to the restarted DBus Daemon the
shared-feature will try to reuse the existing connection even though the
underlying transport already disconnected.

This patch will check the transport connection status on shared
connections before re-using them. If the transport is closed/broken the
stored connection reference will be removed and a new connection is
created. Otherwise if transport is still ok, the same connection is
returned (like before).

README.md

@@ -119,8 +119,9 @@ The library will remain open source and MIT licensed and can still be used, fork
    - Fixed issues with autoConnect option, added method to register to bus by 'Hello' message manually, thanks to [brett-smith](https://github.com/brett-smith) ([#238](https://github.com/hypfvieh/dbus-java/issues/238))
    - Added feature which allows to annotate setter or getter methods in exported objects to use the DBus Properties interface behind the scenes, thanks to [brett-smith](https://github.com/brett-smith) ([PR#235](https://github.com/hypfvieh/dbus-java/issues/235))
    - `ExportedObject.isExcluded(Method)` now returns true for bridge, default and synthetic methods, reported by [brett-smith](https://github.com/brett-smith) ([#240](https://github.com/hypfvieh/dbus-java/issues/240))
-   - DBusViewer: Remove DOCTYPE definition using regex to handle line breaks properly
+   - DBusViewer: Remove DOCTYPE definition in introspection data using a regex which handles line breaks properly
    - Applied changes found by Sonarcloud static code analysis
+   - Fixed issue with shared connections did not work when underlying transport was disconnected due to end-point (daemon) was stopped/restarted ([#244](https://github.com/hypfvieh/dbus-java/issues/244))
 
 ##### Changes in 4.3.1 (2023-10-03):
    - Provide classloader to ServiceLoader in TransportBuilder (for loading actual transports) and AbstractTransport (for loading IMessageReader/Writer implementations), thanks to [cthbleachbit](https://github.com/cthbleachbit) ([#210](https://github.com/hypfvieh/dbus-java/issues/210), [PR#211](https://github.com/hypfvieh/dbus-java/issues/211))

dbus-java-core/src/main/java/org/freedesktop/dbus/connections/impl/DBusConnectionBuilder.java

@@ -168,7 +168,7 @@ public DBusConnection build() throws DBusException {
         if (shared) {
             synchronized (DBusConnection.CONNECTIONS) {
                 String busAddressStr = transportCfg.getBusAddress().toString();
-                c = DBusConnection.CONNECTIONS.get(busAddressStr);
+                c = getSharedConnection(busAddressStr);
                 if (c != null) {
                     c.concurrentConnections.incrementAndGet();
                     return c; // this connection already exists, do not change anything
@@ -187,4 +187,25 @@ public DBusConnection build() throws DBusException {
         return c;
     }
 
+    /**
+     * Retrieve a existing shared connection.
+     * Will remove existing shared connections when underlying transport is disconnected.
+     * @param _busAddr bus address
+     * @return connection if a valid shared connection found or
+     *      null if no connection found or found connection was invalid
+     */
+    private DBusConnection getSharedConnection(String _busAddr) {
+        synchronized (DBusConnection.CONNECTIONS) {
+            DBusConnection c = DBusConnection.CONNECTIONS.get(_busAddr);
+            if (c != null) {
+                if (!c.isConnected()) {
+                    DBusConnection.CONNECTIONS.remove(_busAddr);
+                    return null;
+                } else {
+                    return c;
+                }
+            }
+        }
+        return null;
+    }
 }

dbus-java-tests/src/test/java/sample/issue/Issue244Test.java

@@ -0,0 +1,50 @@
+package sample.issue;
+
+import org.freedesktop.dbus.bin.EmbeddedDBusDaemon;
+import org.freedesktop.dbus.connections.BusAddress;
+import org.freedesktop.dbus.connections.impl.DBusConnectionBuilder;
+import org.freedesktop.dbus.connections.transports.TransportBuilder;
+import org.freedesktop.dbus.test.AbstractBaseTest;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Test case to verify that shared connections do not block reconnect.
+ * When a client uses shared connections and gets disconnected because
+ * the daemon disappears, the shared connection should no longer be used.
+ * Instead this broken connection should be cleaned from the internal connection map.
+ */
+public class Issue244Test extends AbstractBaseTest {
+
+    @Test
+    public void testSharedConnection() {
+        String busType = TransportBuilder.getRegisteredBusTypes().get(0);
+        String addr = TransportBuilder.createDynamicSession(busType, false);
+        BusAddress clientAddress = BusAddress.of(addr);
+        BusAddress serverAddress = BusAddress.of(addr).getListenerAddress();
+
+        logger.info("Creating {} based DBus daemon on address {}", busType, serverAddress);
+
+        try (var edbus = new EmbeddedDBusDaemon(serverAddress)) {
+            edbus.startInBackgroundAndWait(MAX_WAIT);
+            try (var con = DBusConnectionBuilder.forAddress(clientAddress).build()) {
+                assertTrue(con.isConnected(), "First connection attempt must work");
+                edbus.close(); // terminate daemon to enforce disconnect
+                Thread.sleep(1000L);
+                assertFalse(con.isConnected(), "No connection expected after first disconnect"); // no connection without daeon
+            }
+
+            // restart daemon and retry
+            edbus.startInBackgroundAndWait(MAX_WAIT);
+            try (var con = DBusConnectionBuilder.forAddress(clientAddress).build()) {
+                assertTrue(con.isConnected(), "Second connection attempt must work");
+                edbus.close(); // terminate daemon to enforce disconnect
+                Thread.sleep(1000L);
+                assertFalse(con.isConnected(), "No connection expected after second disconnect"); // no connection without daeon
+            }
+
+        } catch (Exception _ex) {
+            fail("No exception expected", _ex);
+        }
+
+    }
+}