hypfvieh/dbus-java

Crash on returning list of nested objects.

brett-smith opened this issue · 9 comments

I'm seeing a connection crash when attempting to access an exported list of objects.

I have distilled the problem from my application into a short, self contained example.

You'll see that getPartNames() works (a list of strings), but getParts() fails (a list of actual objects). As I understand it, dbus-java should turn these into ObjectPath native types for you. This does appear to happen, as if I look in d-feet I can see the method signature correctly, but actually calling it results in the connection being terminated.

It didn't seem to matter if I exported the "parts" myself, the result was the same.

Am I doing it wrong or is this a bug?

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;

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

public class ExportNested {
	public static void main(String[] args) throws Exception {
		try (DBusConnection conn = DBusConnectionBuilder.forSessionBus().build()) {
			var part1 = new MyObjectPart();
			part1.setVal1("ABC");
			part1.setVal2("123");

			var part2 = new MyObjectPart();
			part2.setVal1("DEF");
			part2.setVal2("456");
			
			var myIface = new MyObject();
			myIface.getParts().addAll(Arrays.asList(part1, part2));
			
			conn.requestBusName("com.acme");
			conn.exportObject(part1);
			conn.exportObject(part2);
			conn.exportObject(myIface);
			
			try (DBusConnection innerConn = DBusConnectionBuilder.forSessionBus().build()) {
				var myObject = innerConn.getRemoteObject("com.acme", "/com/acme/MyObject", MyInterface.class);
				
				// Works
				System.out.println("> " + myObject.sayHello());
				
				// Works
				for(String part : myObject.getPartNames()) {
					System.out.println("  " + part);
				}
				
				// Fails
				for(MyInterfacePart part : myObject.getParts()) {
					System.out.println("  " + part.getObjectPath() + " = " +  part.getVal1() + " / " + part.getVal2());
				}
			}
		}
	}
	
	@DBusInterfaceName("com.acme.MyInterface")
	public interface MyInterface extends DBusInterface {
		
		String sayHello();
		
		List<MyInterfacePart> getParts();
		
		List<String> getPartNames();
	}
	
	public static class MyObject implements MyInterface {
		
		private List<MyInterfacePart> parts = new ArrayList<>();
		
		public String sayHello() {
			return "Hello!";
		}
		
		public List<MyInterfacePart> getParts() {
			return parts;
		}

	    public String getObjectPath() {
	    	return "/com/acme/MyObject";
	    }

		@Override
		public List<String> getPartNames() {
			return parts.stream().map(i -> i.getVal1()).collect(Collectors.toList());
		}
	}

	@DBusInterfaceName("com.acme.MyInterfacePart")
	public interface MyInterfacePart extends DBusInterface {
		
		String getVal1();

		String getVal2();
	}

	public static class MyObjectPart implements MyInterfacePart {
		
		private String val1;
		private String val2;
		
		public String getVal1() {
			return val1;
		}

		public void setVal1(String val1) {
			this.val1 = val1;
		}

		public String getVal2() {
			return val2;
		}

		public void setVal2(String val2) {
			this.val2 = val2;
		}

	    public String getObjectPath() {
	    	return "/com/acme/MyPart" + val1;
	    }
	}

This results in ..

> Hello!
  ABC
  DEF
16:15:25.657 [DBusConnection [listener=false]] ERROR o.f.d.c.IncomingMessageThread - FatalException in connection thread
org.freedesktop.dbus.exceptions.FatalDBusException: java.io.EOFException: Underlying transport returned EOF (1)
	at org.freedesktop.dbus.connections.AbstractConnection.readIncoming(AbstractConnection.java:1085)
	at org.freedesktop.dbus.connections.IncomingMessageThread.run(IncomingMessageThread.java:41)
Caused by: java.io.EOFException: Underlying transport returned EOF (1)
	at org.freedesktop.dbus.spi.message.InputStreamMessageReader.readMessage(InputStreamMessageReader.java:52)
	at org.freedesktop.dbus.connections.transports.AbstractTransport.readMessage(AbstractTransport.java:85)
	at org.freedesktop.dbus.connections.AbstractConnection.readIncoming(AbstractConnection.java:1074)
	... 1 common frames omitted

image

This one is tough... As far as I've seen the problem is created when the method return is constructed.

Message.appendOne is called recursively to convert the object list returned by getParts() to the proper DBus representation.
When appendOne is called for the content of the list, it checks the signature type which is 'o' (ObjectPath) and then decides to call toString() on the objects instead of using the object path.

I fixed this right now, added a test and also took you code to provide a working sample in dbus-java-examples project.

Close, but not quite :)

This works if the exported object and the client are in the same JVM. However, I found that when I applied this fix in my application (with 2 JVMs), it failed.

I proved the same thing in the test application above by splitting it into two separate classes, one doing the export and one using it, and it does indeed crash.

> Hello!
  ABC
  DEF
Exception in thread "main" java.lang.ClassCastException: class jdk.proxy2.$Proxy22 cannot be cast to class com.github.hypfvieh.dbus.examples.nested.data.MyInterfacePart (jdk.proxy2.$Proxy22 is in module jdk.proxy2 of loader 'app'; com.github.hypfvieh.dbus.examples.nested.data.MyInterfacePart is in unnamed module of loader 'app')
	at com.github.hypfvieh.dbus.examples.nested.ExportNestedPt2.main(ExportNestedPt2.java:41)

It appears that the proxy object returned does not implement MyInterfacePart.

Edit: It looks like this is happening because @DBusInterfaceName is being used, and the interface name in DBus does not match the Java interface interface name. The code in DBusConnection.dynamicProxy() does not seem to take this into account.

Here is a patch for this. Instead of deriving the type from the introspection data, it needs to derive the type from the Java interface which is known at this time. I wasn't sure if this should always do this, or if it should only do so when the type is not found any other way. I went with always.

dynamic-proxy-type.patch.txt

Nice, I reviewed and applied you patch to master.

That looks good.

I've just found another problem though that seems to be kind of related. It seems that I have a Struct that is not getting deserialised when it is part of a DBusSignal constructor.

I can see the following in the logs ..

2022-03-22 22:03:48.819 WARNING [org.freedesktop.dbus.messages.DBusSignal] (org.freedesktop.dbus.messages.DBusSignal createReal) Could not find suitable constructor for class com.logonbox.containertools.lib.backup.BackupService$BackupNewEntry with argument-types: [class [Ljava.lang.Object;] 

I'll dig a bit deeper and post back.

Edit: Think i've found this. The problem is DBusSignal.matchesParameters(). It is not taking into account that a Struct is presented as an Object[]. So not actually related to the original bug, just another deserialisation issue.

By changing matchesParameters() to ..

public boolean matchesParameters(List<Class<?>> _wantedArgs) {
    if (parameterTypes != null && _wantedArgs == null) {
        return false;
    }
    if (parameterTypes.size() != _wantedArgs.size()) {
        return false;
    }

    for (int i = 0; i < parameterTypes.size(); i++) {
        Class<?> class1 = parameterTypes.get(i);

        /* Structs are presented as object arrays */
        if(Struct.class.isAssignableFrom(class1) && Object[].class.equals(_wantedArgs.get(i))) {
            continue;
        }
         
        if (!class1.isAssignableFrom(_wantedArgs.get(i))) {
            return false;
        }
    }

    return true;
}

.. I get a bit further. Seems there is more to do though.

Edit: This does actually fix it for me. Im not 100% confident it won't break something else though.

Edit: More on this. It seems that signals are not converting DBusInterface implementations either.

Edit: The fix for this was in the same place.

     if(DBusInterface.class.isAssignableFrom(class1) && ObjectPath.class.equals(_wantedArgs.get(i))) {
        continue;
    }

Please provide a PR if you are done with your changes.

Will do. Ive just found that my previous Enum support doesn't work in signals either. Will get that fixed and all of the above in a new PR (along with the tests).

Have bundled all the above changes in #164, which I was in the middle of anyway. It all got a bit complicated, see the PR.

Nice. I'll close this issue.