Something a bit wrong with @DBusBoundProperty

[brett-smith]

There appears to be something fundamentally wrong with @DBusBoundProperty. I can't quite put my finger on it yet, but am getting closer.

All seems well if I am importing an object using these annotations into dbus-java from a service that is also using dbus-java to export it (both ends are using the same interface classes but this doesn't matter).

However, if I try to import interfaces into dbus-java from a service exported by something other than dbus-java, it fails.

The following example fails.

public class AccountsTest {
	
	public static void main(String[] args) throws Exception  {
		try(var conx = DBusConnectionBuilder.forSystemBus().build()) {
			var accounts = conx.getRemoteObject("org.freedesktop.Accounts", "/org/freedesktop/Accounts", Accounts.class);
			System.out.println(accounts.getDaemonVersion());
		}
	}
	
	@DBusInterfaceName("org.freedesktop.Accounts")
	public interface Accounts extends DBusInterface, Properties {
	
		@DBusBoundProperty
		public String getDaemonVersion();
	}
	
}

Results in the exception ...

Exception in thread "main" org.freedesktop.dbus.errors.UnknownMethod: No such method “Get”
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:480)
	at org.freedesktop.dbus.messages.Error.getException(Error.java:111)
	at org.freedesktop.dbus.messages.Error.throwException(Error.java:138)
	at org.freedesktop.dbus.RemoteInvocationHandler.executeRemoteMethod(RemoteInvocationHandler.java:237)
	at org.freedesktop.dbus.RemoteInvocationHandler.executeRemoteMethod(RemoteInvocationHandler.java:250)
	at org.freedesktop.dbus.propertyref.PropRefRemoteHandler.handleDBusBoundProperty(PropRefRemoteHandler.java:69)
	at org.freedesktop.dbus.RemoteInvocationHandler.invoke(RemoteInvocationHandler.java:94)
	at jdk.proxy2/jdk.proxy2.$Proxy4.getDaemonVersion(Unknown Source)
	at test.AccountsTest.main(AccountsTest.java:14)

Now I'm sure this used to work during the initial development, but I cannot find any changes since that may have broken it, so maybe it never actually worked and I only ever tested with dbus-java on both ends.

I wondered if part of the problem was what was addressed by this PR. #252. However, fixing that made no difference.

Digging deeper, I found the following two changes seem to fix it. I've not made a PR for this yet, as this was just me stabbing randomly at the keyboard until it worked, and it probably breaks other stuff.

This is in PropRefRemoteHandler. It basically changes the interface that is held by RemoteObject to Properties. I am fairly happy this is a correct change, and safe, as it only applies when the annotation is in use.

The second change was in Marshalling. The was necessary because the return value was always coming back as a Variant in this case. This one was a bit brute force, and looks likely to break something else. But it should hopefully be a clue as to what is actually going on.

The fact that it works with dbus-java exporting the service is a bit odd. What is it doing differently I wonder? It seems like it might not be following the specification of Properties properly, and the importing side was incorrectly designed to match that.

[hypfvieh]

I've looked at this case and I agree that something is missing here.

I first investigated what you mentioned about PropRefRemoteHandler.
When the method handleDBusBoundProperty is used it is assumed that Properties is supported by the given remote object.

That is wrong. The remote object the method receives is an arbitrary object which is not necessarily related to Properties.
In most cases the object behind the remote path is supporting Properties but not necessarily the object passed to this method.

So your fix to create another remote object using Properties will work because the calls are delegated to the object behind the path, not to the given object itself.

For me this seems to be a valid fix. Some tests would be great, but how to test a thing which did work before?

The second thing you mentioned is Marshalling class.
I have to admit I don't see why there is a need to change anything. Also I'm not sure what your change should do. When debugging this the parameter is not Variant it is String so the code will never be executed.

The right fix for this is again in PropRefRemoteHandler.
When handleDBusBoundProperty has executed the remote method call, it must take care of the result.

What is happening here?
The remote call will (hopefully) resolve the requested property and as the javadoc on that method already says, all properties on DBus are always Variant.
Variant may not be the type of object we want to return here. This depends on how the @DBusBoundProperty was used (e.g. the return of the method is Variant).

So we may have to unwrap the Variant here to get the correct object type:

Object result = null;

if (access == Access.READ) {
    result = RemoteInvocationHandler.executeRemoteMethod(propertiesRemoteObj, PROP_GET_METHOD,
            new Type[] {_method.getGenericReturnType()}, _conn, RemoteInvocationHandler.CALL_TYPE_SYNC, null, DBusNamingUtil.getInterfaceName(_method.getDeclaringClass()), name);
} else {
    result = RemoteInvocationHandler.executeRemoteMethod(propertiesRemoteObj, PROP_SET_METHOD, variantType,
            new Type[] {_method.getGenericReturnType()}, _conn, RemoteInvocationHandler.CALL_TYPE_SYNC, null, DBusNamingUtil.getInterfaceName(_method.getDeclaringClass()), name, _args[0]);
}

// requested return type is not Variant but the result is -> unwrap Variant
if (_method.getReturnType() != Variant.class && typeClass != Variant.class && result instanceof Variant<?> v) {
    return v.getValue();
}

return result;

I've already updated master branch to include this patch and your patch as well.

[hypfvieh]

I did some more work on this stuff.
I found that there are some combinations of implementing Properties and using @DBusBoundProperty annotation which will not work or will not produce the expected results.
Especially some instanceof checks with @DBusProperties annotations were definitely wrong as no object exported on/to the bus can/will ever be a subclass of an annotation.

All changes are already in master branch so you can try it when the new snapshot is deployed to snapshot repository.

[hypfvieh]

@brett-smith Any update on this issue? I'm planning to create a release in the next weeks and want to ensure that this issue is fixed.

[brett-smith]

Apologies, I did see the original comment, made a mental note to check it out and almost immediately forgot. However, I have been very busy using the snapshot for the past months and its all been fine.

I will double check now though to make sure I am using that SNAPSHOT, as I have vague memories of using my own SNAPSHOT at one point.

[brett-smith]

Confirmed I have been using the oss-snapshots version for the past few months, and everything is acting as I'd expect. The annotation is used extensively, for various different types, with both dbus-java based exported services, and native ones. And the original problem is still gone too.

So, all good. Mark this one as fixed.

[brett-smith]

Hrm. Well ... I have another project that ive not touched for a few months, but is now behaving rather strangely with the latest snapshots. I am seeing strange hangs and problems with serialization. It may be something else entirely, will post back when more is known.

[brett-smith]

Ok, I understand what happening in this particular case. Neither are anything do with this ticket, and one is a problem entirely of my own making.

The "random hangs", were caused by #213 (one of my requests in the first place). In this application, I am iterating over multiple possible bus paths looking for a system bus (differs platform to platform). I try to look for the native system bus, and then a custom bus that uses dbus-java, etc. It relies on getting an exception when the connection is built.

However, the effects of 213 means that by default every attempt is going to take 10 seconds, as the bus file will never exist. The work around for this is to either set the timeout to 500ms, or turn off auto connection and deal with it myself. The conflict of interest here seems to be that dbus-java doesn't know if the socket file doesn't exist because it's not ready yet, or if it will never exist. I need to go re-read 231 to refresh memory, but from what I remember it was largely a problem when the embedded daemon was used.

The other problem was of my own making. I was using D-Feet to inspect my own exported server, and certain objects have lots of @DBusBoundProperty. D-Feet displays all properties at once, so every single bound property was being called. Some of these properties were computed at runtime, and could take a little time. This was fine when each property was accessed individually, but the cumulative effect of all properties being access resulted in timeouts.

So this particular app was upgraded from the legacy method of properties to the new annotation a little thoughtlessly.