mono / dbus-sharp

DBus Sharp
http://mono.github.com/dbus-sharp
MIT License
77 stars 59 forks source link

proxy: Enable DBus property extraction via CLR properties #37

Closed arfbtwn closed 7 years ago

arfbtwn commented 10 years ago

Properties were previously involved in some name mangling to separate them from normal methods, events were dealt with in a similar way. As result, although signals worked correctly, properties did not.

This patch flips the implementation process on its head, first building a set of methods for evaluation and removing pairs as properties or signals (events) are implemented before passing the remainder to the general function.

Property Get/Set is handled by two additional functions in the BusObject class, the IL is generated specifically for them. Event listener addition and removal IL generation is refactored into its own function in the same way.

arfbtwn commented 10 years ago

When I said that signals worked ok, it seems I was also mistaken-at least for events implemented on interfaces originating from F#. An error occurs while generating the hookup method when inferring the signature as shown below. The first commit above, to map CLR properties to DBus ones, inadvertently corrects the issue through separation of Event and Property methods from the remaining ones.

Unhandled Exception:
System.ArgumentException: Cannot create a struct with no fields
Parameter name: signature
  at DBus.Protocol.Signature.MakeStruct (Signature signature) [0x00000] in <filename unknown>:0 
  at DBus.Protocol.Signature.GetSig (System.Type type) [0x00000] in <filename unknown>:0 
  at DBus.TypeImplementer.SigsForMethod (System.Reflection.MethodInfo mi, DBus.Protocol.Signature& inSig, DBus.Protocol.Signature& outSig) [0x00000] in <filename unknown>:0 
  at DBus.TypeImplementer.GenHookupMethod (System.Reflection.Emit.ILGenerator ilg, System.Reflection.MethodInfo declMethod, System.Reflection.MethodInfo invokeMethod, System.String interface, System.String member) [0x00000] in <filename unknown>:0 
  at DBus.TypeImplementer.Implement (System.Reflection.Emit.TypeBuilder typeB, System.Type iface, System.String interfaceName) [0x00000] in <filename unknown>:0 
  at DBus.TypeImplementer.GetImplementation (System.Type declType) [0x00000] in <filename unknown>:0 
  at DBus.BusObject.GetObject (DBus.Connection conn, System.String bus_name, DBus.ObjectPath object_path, System.Type declType) [0x00000] in <filename unknown>:0 
  at DBus.Connection.GetObject (System.Type type, System.String bus_name, DBus.ObjectPath path) [0x00000] in <filename unknown>:0 
  at DBus.Connection.GetObject[IObjectManager] (System.String bus_name, DBus.ObjectPath path) [0x00000] in <filename unknown>:0 
  at <StartupCode$helloobex>.$Obex.main@ () [0x00000] in <filename unknown>:0 
chmorgan commented 9 years ago

Looking to get property support merged into mono mainline dbus-sharp.

arfbtwn commented 9 years ago

Cool, however since I made this request I've polluted it by having it point to my master. I'll do what cleanup I can in the morning-perhaps closing this and opening a new one.

chmorgan commented 9 years ago

Cool. I thought this was going to bug the mono guys to look at integrating the pull request, not you :-)

We are using this at my company to add property support as mainline dbus-sharp doesn't support it. Appears to be working pretty well so thought it was worth a ping to help get it merged.

arfbtwn commented 9 years ago

@chmorgan Don't worry, we are bugging them ;) but I get notifications too because I created the pull.

I took a look at the series of commits and there's only really two that aren't necessary for property get and set since it's based upon some refactoring I did to fix a bug with using interfaces specified with F# (Pull #39).

However, properties could be done without the refactoring (though they're a whole lot easier with it) so they may want to close that request first before merging this, we'll see.

Fantastic to hear that someone's finding these changes useful! I was thinking of adding some caching in the BusObject a while ago so that property access won't always need to send a DBus message, have you noticed any performance issues?

chmorgan commented 9 years ago

HI @arfbtwn. We haven't actually seen any performance issues thus far. We aren't retrieving properties all that often at this point, just at application startup.

Appreciate the efforts in getting this merged in. We are using the code after your rebase and changes and it still seems like everything is working so :+1:

chmorgan commented 9 years ago

Hmm. I'm seeing exceptions here when calling a method via dbus. Using rev 3bf609:

[ERROR] FATAL UNHANDLED EXCEPTION: System.IndexOutOfRangeException: Array index is out of range. service[25636]: at System.Threading.Thread.StartInternal () [0x00000] in :0 service[25636]: at service.MainClass.

m__0 () [0x00000] in :0 service[25636]: at DBus.Connection.Iterate () [0x00000] in :0 service[25636]: at DBus.Connection.HandleMessage (DBus.Protocol.Message msg) [0x00000] in :0 service[25636]: at DBus.Connection.HandleMethodCall (DBus.Protocol.MessageContainer method_call) [0x00000] in service[25636]: at DBus.ExportObject.HandleMethodCall (DBus.Protocol.MessageContainer method_call) [0x00000] in <filename unknow service[25636]: at DBus.ExportObject.HandlePropertyCall (DBus.Protocol.MessageContainer method_call) [0x00000] in <filename unkn service[25636]: System.IndexOutOfRangeException: Array index is out of range.

chmorgan commented 9 years ago

Different crash now with 50ea4570, and with --debug to produce some useful output:

caught System.NullReferenceException: Object reference not set to an instance of an object at DBus.TypeImplementer.GenGetAllCall (System.Type interface) [0x0009e] in service/dbus-sharp/src/TypeImplementer.cs:640 at DBus.DBusMemberTable.GetPropertyAllCall (System.String iface) [0x00044] in service/dbus-sharp/src/ExportObject.cs:73 at DBus.ExportObject.HandlePropertyCall (DBus.Protocol.MessageContainer method_call) [0x00064] in service/dbus-sharp/src/ExportObject.cs:260 at DBus.ExportObject.HandleMethodCall (DBus.Protocol.MessageContainer method_call) [0x00025] in service/dbus-sharp/src/ExportObject.cs:175 at DBus.Connection.HandleMethodCall (DBus.Protocol.MessageContainer method_call) [0x00241] in service/dbus-sharp/src/Connection.cs:394 at DBus.Connection.HandleMessage (DBus.Protocol.Message msg) [0x00113] in service/dbus-sharp/src/Connection.cs:264 at DBus.Connection.Iterate () [0x0000f] in service/dbus-sharp/src/Connection.cs:223 at simulator.MainClass.Main (System.String[] args) [0x00225] in service/simulator/Main.cs:326

So some background, I'm using dbus-sharp to expose some methods from c# code. Only methods, no properties. And I'm using d-feet to call one of those methods that returns two values copied from an internal structure and a status, so nothing complex.

chmorgan commented 9 years ago

I'm also seeing unit test failures with 50ea4570:

Runtime Environment - OS Version: Unix 3.18.7.200 CLR Version: 2.0.50727.1433 ( 3.12.0 (tarball Fri Jan 16 19:45:30 UTC 2015) )

....................F.F...................................................... Tests run: 75, Failures: 2, Not run: 0, Time: 1.969 seconds

Test Case Failures: 1) DBus.Tests.IntrospectorTest.InterfaceThroughWireTest : Expected: True But was: False

at DBus.Tests.IntrospectorTest.InterfaceThroughWireTest () [0x00054] in /home/cmorgan/projects/ikabit/services/motor_lcb/dbus-sharp/tests/IntrospectorTest.cs:95 at (wrapper managed-to-native) System.Reflection.MonoMethod:InternalInvoke (System.Reflection.MonoMethod,object,object[],System.Exception&) at System.Reflection.MonoMethod.Invoke (System.Object obj, BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00054] in /builddir/build/BUILD/mono-3.12.0/mcs/class/corlib/System.Reflection/MonoMethod.cs:230

2) DBus.Tests.IntrospectorTest.SimpleInterfaceTest : Expected: True But was: False

at DBus.Tests.IntrospectorTest.SimpleInterfaceTest () [0x00031] in /home/cmorgan/projects/ikabit/services/motor_lcb/dbus-sharp/tests/IntrospectorTest.cs:80 at (wrapper managed-to-native) System.Reflection.MonoMethod:InternalInvoke (System.Reflection.MonoMethod,object,object[],System.Exception&) at System.Reflection.MonoMethod.Invoke (System.Object obj, BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00054] in /builddir/build/BUILD/mono-3.12.0/mcs/class/corlib/System.Reflection/MonoMethod.cs:230

arfbtwn commented 9 years ago

Don't worry about the unit tests, they're a check against a introspection response which contains the library version, only the test wasn't updated when the library version was bumped.

As for the other crash, I was expecting it to have been a failure getting the interface type-but it looks like it's a null reference result from typeof(Dictionary<string,object>) but that would have been triggered at line 639, right? So let's try creating that generic type ourselves and throwing something arbitrary we can be sure about to find out what's going on.

chmorgan commented 9 years ago

How can I better help debug this? Make a test case or are we doing well already?

On Sun, Feb 22, 2015 at 11:06 AM, Nicholas notifications@github.com wrote:

Don't worry about the unit tests, they're a check against a introspection response which contains the library version, only the test wasn't updated when the library version was bumped.

As for the other crash, I was expecting it to have been a failure getting the interface type-but it looks like it's a null reference result from typeof(Dictionary). So let's try creating that generic type ourselves and throwing something arbitrary we can be sure about to find out what's going on.

— Reply to this email directly or view it on GitHub https://github.com/mono/dbus-sharp/pull/37#issuecomment-75442279.

arfbtwn commented 9 years ago

Apply the last two commits and let's see another stack trace. :-)

chmorgan commented 9 years ago

Same method call, same app, call via d-feet from 580c4a:

caught System.ArgumentNullException: Argument cannot be null.

Parameter name: interface

at DBus.TypeImplementer.GenGetAllCall (System.Type interface) [0x00281] in /service/dbus-sharp/src/TypeImplementer.cs:691

at DBus.DBusMemberTable.GetPropertyAllCall (System.String iface) [0x00044] in /service/dbus-sharp/src/ExportObject.cs:73

at DBus.ExportObject.HandlePropertyCall (DBus.Protocol.MessageContainer method_call) [0x00064] in /service/dbus-sharp/src/ExportObject.cs:261

at DBus.ExportObject.HandleMethodCall (DBus.Protocol.MessageContainer method_call) [0x00025] in /service/dbus-sharp/src/ExportObject.cs:176

at DBus.Connection.HandleMethodCall (DBus.Protocol.MessageContainer method_call) [0x00241] in /service/dbus-sharp/src/Connection.cs:394

at DBus.Connection.HandleMessage (DBus.Protocol.Message msg) [0x00113] in /service/dbus-sharp/src/Connection.cs:264

at DBus.Connection.Iterate () [0x0000f] in /service/dbus-sharp/src/Connection.cs:223

at simulator.MainClass.Main (System.String[] args) [0x00225] in /service/simulator/Main.cs:326

Unhandled Exception:

System.ArgumentNullException: Argument cannot be null.

Parameter name: interface

at DBus.TypeImplementer.GenGetAllCall (System.Type interface) [0x00281] in /service/dbus-sharp/src/TypeImplementer.cs:691

at DBus.DBusMemberTable.GetPropertyAllCall (System.String iface) [0x00044] in /service/dbus-sharp/src/ExportObject.cs:73

at DBus.ExportObject.HandlePropertyCall (DBus.Protocol.MessageContainer method_call) [0x00064] in /service/dbus-sharp/src/ExportObject.cs:261

at DBus.ExportObject.HandleMethodCall (DBus.Protocol.MessageContainer method_call) [0x00025] in /service/dbus-sharp/src/ExportObject.cs:176

at DBus.Connection.HandleMethodCall (DBus.Protocol.MessageContainer method_call) [0x00241] in /service/dbus-sharp/src/Connection.cs:394

at DBus.Connection.HandleMessage (DBus.Protocol.Message msg) [0x00113] in /service/dbus-sharp/src/Connection.cs:264

at DBus.Connection.Iterate () [0x0000f] in /service/dbus-sharp/src/Connection.cs:223

at simulator.MainClass.Main (System.String[] args) [0x00225] in /service/simulator/Main.cs:326

On Sun, Feb 22, 2015 at 1:04 PM, Nicholas notifications@github.com wrote:

Apply the last two commits and let's see another stack trace. :-)

— Reply to this email directly or view it on GitHub https://github.com/mono/dbus-sharp/pull/37#issuecomment-75448426.

arfbtwn commented 9 years ago

OK, so it looks like d-feet is requesting an interface name without an equivalent on your exported object. If I'm right, the last change should get you up and running again and I can tidy it up later.

chmorgan commented 9 years ago

Using 7ac412e there aren't any exceptions thrown that are showing up as uncaught but the calls to the dbus methods are failing with:

'g-dbus-error-quark: GDBus.Error:org.freedesktop.DBus.Error.UnknownMethod: Method "GeFrequency" with signature "" on interface "com.simulator" doesn\'t exist (19)'

I'm pretty sure that this was working well last week with b09350d51f1fc01b295c4 which I guess was part of a rebased branch that no longer exists. We started pulling in 3bf609679c76ac and I did a full rebuild and started to see the issues. I know that this was working under mono/dbus-sharp master but another developer here wanted to expose object manager interfaces and needed properties to do it so he started using your branch (b09350)

codyps commented 9 years ago

"GeFrequency" spelling, prehaps? Or if this is invoked via d-feet, maybe something in the introspection data isn't quite right?

chmorgan commented 9 years ago

Hi Cody. I've been editing the output for confidentialities sake. Cody is a co-worker :-)

I'm seeing the same issue here with the service side, any method. Are you seeing the same thing there if you rebuild things and try the interfaces via d-feet?

And by same thing I mean you could try known working method calls on the service side, like Init()

chmorgan commented 9 years ago

@arfbtwn do you need a test case for this functionality to continue? It looks like you've mitigated the unhandled exceptions but that the calls aren't functional.

arfbtwn commented 9 years ago

@chmorgan: I have a few thoughts about what's going on but yes, a test case would be supremely useful. Also, my apologies for the various history rewrites. Your co-worker probably wants the branch called properties from my repository.

chmorgan commented 9 years ago

https://github.com/chmorgan/dbussharptest

single method, reproduces here for me with 7ac412e when called via d-feet

On Mon, Feb 23, 2015 at 2:09 PM, Nicholas notifications@github.com wrote:

@chmorgan https://github.com/chmorgan: I have a few thoughts about what's going on but yes, a test case would be supremely useful.

— Reply to this email directly or view it on GitHub https://github.com/mono/dbus-sharp/pull/37#issuecomment-75609608.

arfbtwn commented 9 years ago

Great! That's totally different from what I was expecting. If you modify the program to retrieve a proxy after exporting it I'm guessing calling the proxy's method fails too?

chmorgan commented 9 years ago

I'm not sure what you are asking for here. You mean creating an object to be a client of the exported class?

On Mon, Feb 23, 2015 at 4:27 PM, Nicholas notifications@github.com wrote:

Great! That's totally different from what I was expecting. If you modify the program to retrieve a proxy after exporting it I'm guessing calling the proxy's method fails too?

— Reply to this email directly or view it on GitHub https://github.com/mono/dbus-sharp/pull/37#issuecomment-75637524.

arfbtwn commented 9 years ago

Don't worry, I'll have a good look at the test case on Wednesday night.

chmorgan commented 9 years ago

Cool. Let me know if I can help out with testing.

On Mon, Feb 23, 2015 at 5:40 PM, Nicholas notifications@github.com wrote:

Don't worry, I'll have a good look at the test case on Wednesday night.

— Reply to this email directly or view it on GitHub https://github.com/mono/dbus-sharp/pull/37#issuecomment-75653566.

chmorgan commented 9 years ago

Working much better after 50cee72, a handful of calls I tried are working without issue. Looks like that resolved it.

arfbtwn commented 9 years ago

That's great!

Don't worry about it, it's a perfectly valid setup. Since you use d-feet to call methods on your exported objects instead of using Bus.GetObject within a program I don't think it's a problem for you.

It's definitely recommended when using the above call, since dbus-sharp can implement interfaces at runtime instead of going through the RealProxy route with DProxy.

I've added a short unit test in a new branch, which shows some problems I'll need to look at, at some point...

mkcybi commented 9 years ago

Hi @arfbtwn - I seem to be having an issue with this pull request.

We've been using your branch of dbus-sharp and seem to be getting an unhandled exception when we try to add an interface to one of our classes.

ProtSystem.Exception: Message length 993591296 exceeds maximum allowed 134217728 bytes ocolInformation.MaxMessageLength + " bytes")

Please let me know if you need more information. Stack trace

Revision 50cee72 (HEAD, origin/master, origin/HEAD, master) mapper: Correct GetInterfaceType(Type,string) [Nicholas Little]

DBus.Transports.Transport.ReadMessageReal () in /home/user/product/services/service/dbus-sharp/src/Protocol/Transport.cs:233 DBus.Transports.Transport.ReadMessage () in /home/user/product/services/service/dbus-sharp/src/Protocol/Transport.cs:155 DBus.Protocol.PendingCall.get_Reply () in /home/user/product/services/service/dbus-sharp/src/Protocol/PendingCall.cs:31 DBus.Connection.SendWithReplyAndBlock (msg={DBus.Protocol.Message}) in /home/user/product/services/service/dbus-sharp/src/Connection.cs:177 DBus.BusObject.SendMethodCall (iface="org.freedesktop.DBus", member="AddMatch", inSigStr="s", writer={DBus.Protocol.MessageWriter}, retType={System.Void}, exception=(null)) in /home/user/product/services/service/dbus-sharp/src/BusObject.cs:141 org.freedesktop.DBus.IBusProxy.AddMatch (rule="type='signal',path='/com/company/devices',interface='org.freedesktop.ObjectManager',member='Interfaces…") in DBus.Bus.AddMatch (rule="type='signal',path='/com/company/devices',interface='org.freedesktop.ObjectManager',member='Interfaces…") in /home/user/product/services/service/dbus-sharp/src/Bus.cs:141 DBus.BusObject.ToggleSignal (iface="org.freedesktop.ObjectManager", member="InterfacesAdded", dlg={service.InterfacesAddedHandler}, adding=true) in /home/user/product/services/service/dbus-sharp/src/BusObject.cs:72 service.ObjectManagerProxy.add_InterfacesAdded (value={service.InterfacesAddedHandler}) in service.FindDeviceName.Init () in /home/user/product/services/service/service/FindDeviceName.cs:43 service.MainClass.Main (Args={string[0]}) in /home/user/product/services/service/service/Main.cs:316

This is the definition of object manager

    public delegate void InterfacesAddedHandler(ObjectPath object_path,             IDictionary<string, IDictionary<string,object>> interfaces_and_properties);

    public delegate void InterfacesRemovedHandler(ObjectPath object_path, string[] interfaces);

    [Interface("org.freedesktop.ObjectManager")]     public interface ObjectManager     {         void GetManagedObjects(out IDictionary<ObjectPath,                 IDictionary<string,IDictionary<string,object>>> objpath_interfaces_and_properties);         event InterfacesAddedHandler InterfacesAdded;         event InterfacesRemovedHandler InterfacesRemoved;     }

arfbtwn commented 9 years ago

Oops! Was signed in on my work account... @mkcybi: Please open an issue against my fork and we'll discuss it there, I'd like to keep this pull request as free from non-property issues as possible. Cheers!

arfbtwn commented 7 years ago

Old and stale, moved to #66.