`default` interface methods. Or not.

[brett-smith]

I have a situation where I am sharing some code across a few modules, and my DBusInterface interface and implementation extend a generic interface that is used outside of the dbus components.

Something like this ..

public interface RootItem {
}

public interface Root<ITEM_TYPE extends RootItem> {
     public ITEM_TYPE getItem(long itemId);
}

public interface DBusItem extends RootItem {
}

public interface DBusAPI extends Root<DBusItem>, DBusInterface {
    @Override
    public DBusItem getItem(long itemId);
}

public class DBusAPIImpl implements DBusAPI {
    @Override
    public DBusItem getItem(long itemId) {
         /// do stuff to get item
         return theItem;
    }
}

This will fail to export the interface.

org.freedesktop.dbus.exceptions.DBusException: Exporting non-exportable type: interface com.logonbox.vpn.client.common.api.IVPNConnection
	at [email protected]/org.freedesktop.dbus.Marshalling.recursiveGetDBusType(Marshalling.java:281)
	at [email protected]/org.freedesktop.dbus.Marshalling.getDBusType(Marshalling.java:118)
	at [email protected]/org.freedesktop.dbus.Marshalling.getDBusType(Marshalling.java:103)
	at [email protected]/org.freedesktop.dbus.messages.ExportedObject.generateMethodsXml(ExportedObject.java:227)
	at [email protected]/org.freedesktop.dbus.messages.ExportedObject.generateIntrospectionXml(ExportedObject.java:341)
	at [email protected]/org.freedesktop.dbus.messages.ExportedObject.<init>(ExportedObject.java:37)
	at [email protected]/org.freedesktop.dbus.connections.AbstractConnection.exportObject(AbstractConnection.java:329)

What appears to be happening, is that the getItem() method is being found twice. What is is interesting, is that one occurrence is being identified as a default method even though it's not, while the other is being identified as abstract (like all the other methods in the interface that do not have this generic type).

Doing a System.out of the two methods that are found results in ...

public default com.acme.root.RootItem com.acme.impl.DBusAPI.getItemlong)
public abstract com.acme.impl.DBusItem com.acme.impl.DBusAPI.getItem(long)

I could fix this by adding a @DBusIgnore to Root, but Root is in a module that does not have dbus-java on the CLASSPATH.

So instead, I changed dbus-javas ExportedObject.isExcluded() method.

 public static boolean isExcluded(Method _meth) {
        return !Modifier.isPublic(_meth.getModifiers())
                || _meth.isDefault() /* <--- Added this */
                || _meth.getAnnotation(DBusIgnore.class) != null
                || _meth.getAnnotation(DBusBoundProperty.class) != null
                || _meth.getName().equals("getObjectPath") && _meth.getReturnType().equals(String.class)
                        && _meth.getParameterCount() == 0;
    }

And now it all works again.

This doesn't seem to have any other ill effects to the DBus API. Everything else is still getting exported correctly. It seems that it is only identified as default with this particular arrangement of interfaces, OR if you do actually use a default method in the DBusInterface.

I think this may be a valid fix, because using a default method doesn't make sense anyway. If you are acting as a client, accessing an external DBus service from Java, then default methods don't work anyway (RemoteInvocationHandler tries to run it as if it were a remote method). It might be valid if you are exporting an API from Java, but then you cannot share those interface definitions with client code.

TLDR; I think default methods in DBus interfaces should be excluded by default

[hypfvieh]

Can you create a example demonstrating this? I would like to understand where the "default" implementation comes from...

[brett-smith]

That is what is strange, because I haven't provided one. I'll get together a self contained example.

[brett-smith]

package com.acme;

import org.freedesktop.dbus.connections.impl.DBusConnection;
import org.freedesktop.dbus.connections.impl.DBusConnectionBuilder;
import org.freedesktop.dbus.interfaces.DBusInterface;

public class DefaultInterfaceMethodTest {

    public final static void main(String[] args) throws Exception {
        try (DBusConnection conn = DBusConnectionBuilder.forSessionBus().withShared(false).build()) {
            DBusAPI obj = new DBusAPIImpl();

            conn.requestBusName("com.acme");
            conn.exportObject(obj);
           
            Thread.sleep(Integer.MAX_VALUE);

        }
    }
    
    /* The following two interface in the real world exist in a separate
     * library that does not link to dbus in any way
     */
    
    public interface RootItem {
    }

    public interface Root<ITEM_TYPE extends RootItem> {
         ITEM_TYPE getItem(long itemId);
    }
    
    
    /* And the remaining 2 interfaces and 1 class here are dbus specific 
     * implementations of the above
     */

    public interface DBusItem extends RootItem, DBusInterface {
    }

    public interface DBusAPI extends Root<DBusItem>, DBusInterface {
        @Override
        DBusItem getItem(long itemId);
    }

    public static class DBusAPIImpl implements DBusAPI {
        @Override
        public DBusItem getItem(long itemId) {
             /// do stuff to get item
             return new DBusItem() {
                @Override
                public String getObjectPath() {
                    return "/com/acme/Items/" + itemId;
                }
             };
        }

        @Override
        public String getObjectPath() {
            return "/com/acme/MyApi";
        }

        @Override
        public boolean isRemote() {
            return true;
        }

    }
}

If you put a System.out.println(meth) in ExportedObject.generateMethodsXml just after the isExcluded(), you'd see ...

public abstract void org.freedesktop.dbus.interfaces.Peer.Ping()
public abstract java.lang.String org.freedesktop.dbus.interfaces.Peer.GetMachineId()
public abstract java.lang.String org.freedesktop.dbus.interfaces.Introspectable.Introspect()
public abstract com.acme.DefaultInterfaceMethodTest$DBusItem com.acme.DefaultInterfaceMethodTest$DBusAPI.getItem(long)
public default com.acme.DefaultInterfaceMethodTest$RootItem com.acme.DefaultInterfaceMethodTest$DBusAPI.getItem(long)
Exception in thread "main" org.freedesktop.dbus.exceptions.DBusException: Exporting non-exportable type: interface com.acme.DefaultInterfaceMethodTest$RootItem
	at org.freedesktop.dbus/org.freedesktop.dbus.Marshalling.recursiveGetDBusType(Marshalling.java:281)
	at org.freedesktop.dbus/org.freedesktop.dbus.Marshalling.getDBusType(Marshalling.java:118)
	at org.freedesktop.dbus/org.freedesktop.dbus.Marshalling.getDBusType(Marshalling.java:103)
	at org.freedesktop.dbus/org.freedesktop.dbus.messages.ExportedObject.generateMethodsXml(ExportedObject.java:228)
	at org.freedesktop.dbus/org.freedesktop.dbus.messages.ExportedObject.generateIntrospectionXml(ExportedObject.java:342)
	at org.freedesktop.dbus/org.freedesktop.dbus.messages.ExportedObject.<init>(ExportedObject.java:37)
	at org.freedesktop.dbus/org.freedesktop.dbus.connections.AbstractConnection.exportObject(AbstractConnection.java:329)
	at org.freedesktop.dbus/org.freedesktop.dbus.connections.AbstractConnection.exportObject(AbstractConnection.java:349)
	at com.acme.DefaultInterfaceMethodTest.main(DefaultInterfaceMethodTest.java:14)

As you can see, there are no default methods.

[hypfvieh]

So after digging into this, I guess you stumbled across a bridging method generated by the compiler. You can see that by asking if the method isSynthetic (or by asking for isBridge), which are both true in your example code.

Those methods will be created when multiple methods will override one another from the language perspective. You may take a look at this stack overflow article See also the Javadoc of Method.isBridge().

So we got three things which needs to be taken care of: default implementations in interfaces, synthetic methods and bridge methods.

I think the isExcluded() method should return true in all cases.

1. As you already stated it doesn't make sense to provide a default implementation when exporting the interface on the bus. The default would not be working in any case.
2. Same goes for synthetic methods. They are not defined on the interface by the developer, so they also should never be exported on DBus.
3. Bridge methods are some sort of special synthetic methods which were also not defined by the developer, so no need to export them as well

[brett-smith]

Ohh interesting. Today I learned something new about Java :)

All the above makes sense.

One further thing ... I was thinking about default methods, and that they may be actually valid in a client only interface. I don't think there would be any harm in changing RemoteInvocationHandler, so that if it encounters a default method, then it should just execute it locally. If this is being ignored now on the server side, that means the interfaces can safely be shared.

[hypfvieh]

Ohh interesting. Today I learned something new about Java :)

Me too. Most of the time one does not really care what is happening under the hood.. but in some rare cases these details make the difference.

One further thing ... I was thinking about default methods, and that they may be actually valid in a client only interface. I don't think there would be any harm in changing RemoteInvocationHandler, so that if it encounters a default method, then it should just execute it locally. If this is being ignored now on the server side, that means the interfaces can safely be shared.

I'm really not so sure what's the best way to deal with default methods. There may be some cases where they may work depending what the user does. If you have a good idea, please provide a PR.

[brett-smith]

Heh yeh, I'm thinking they may be useful for something like this this ..

public interface SomeExternalDBusService extends DBusInterface {
     @DBusBoundProperty
     long getMemoryUsed();

     @DBusBoundProperty
     long getMemoryTotal();

     /* SomeService doesn't provide us with a getMemoryFree(), so we make our own as a convenience to you */
     default long getMemoryFree() {
          return getMemoryTotal() - getMemoryUsed();
     }
}

I'm pretty sure it's no more than a handful of lines of code in RemoteInvocationHandler. I'll get together a PR later.