Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash on returning list of nested objects. #163

Closed
brett-smith opened this issue Mar 19, 2022 · 9 comments
Closed

Crash on returning list of nested objects. #163

brett-smith opened this issue Mar 19, 2022 · 9 comments

Comments

@brett-smith
Copy link
Contributor

brett-smith commented Mar 19, 2022

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

hypfvieh added a commit that referenced this issue Mar 21, 2022
hypfvieh added a commit that referenced this issue Mar 21, 2022
@hypfvieh
Copy link
Owner

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.

@brett-smith
Copy link
Contributor Author

brett-smith commented Mar 21, 2022

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.

@brett-smith
Copy link
Contributor Author

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

@hypfvieh
Copy link
Owner

Nice, I reviewed and applied you patch to master.

@brett-smith
Copy link
Contributor Author

brett-smith commented Mar 22, 2022

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;
    }

@hypfvieh
Copy link
Owner

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

@brett-smith
Copy link
Contributor Author

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).

@brett-smith
Copy link
Contributor Author

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.

@hypfvieh
Copy link
Owner

Nice. I'll close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants