proxy: Enable DBus property extraction via CLR properties #37
Conversation
|
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. |
|
Looking to get property support merged into mono mainline dbus-sharp. |
|
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. |
|
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. |
The MethodInfo.IsSpecialName property doesn't appear to be set true for event add/remove methods produced with the F# compiler, even with the [<CLIEvent>] annotation applied to the event. This appears partly due to F#'s generalisation of the signature for event handler delegate types, but also because we attempt to treat all methods equally when implementing them. This patch provides a quick hack to solve the problem with minimal change to the TypeImplementor class.
Since the previous commit was a hack, and slightly questionable, this one builds upon it by adding a class with two simple extension methods for MethodBase objects; IsEvent and IsProperty. They are used to ensure the correct branches are followed when generating hookup methods for proxy classes since the IsSpecialName property is unreliable on interfaces originating in F#.
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.
The previous patch worked great for object references (e.g. strings) but was generating incorrect IL for properties expressed as value types. This patch adds box and unbox OpCodes for the affected properties.
|
@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? |
This commit: - Corrects the failing Property unit test; - Adds ExportObject.HandlePropertyCall(MessageContainer); - Adds Mapper.GetInterfaceType(Type, string); - Adds Mapper.GetPublicProperties(Type); - Adds a structure, PropertyCallers to hold the property type and a MethodCaller (or null) for the accessor and mutator; - Adds TypeImplementor.GenPropertyCallers(PropertyInfo) which produces the above structure; - Adds IEnumerable<object> MessageReader.ReadValues() method which will yield objects based on the signature of the received Message the MessageReader was constructed with.
arfbtwn
force-pushed
the
master
branch
2 times, most recently
from
b85e9dc to
3bf6096
Compare
My spacing settings convert tabs to spaces, I'm not certain this is the default, but we'll add the specification here to prevent ambiguity.
This commit: - Adds a new type, DBusMemberTable which we'll use to formalise the method and property calling apparatus, compared to using four dictionaries; - Moves some of the uses of TypeImplementer from ExportObject to reduce coupling between the two classes; - Renames some badly named methods in TypeImplementer; - Corrects erroneous spacing in previous commits.
|
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 👍 |
|
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 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. |
This commit adds a prototype GetAll response and accompanying unit test to demonstrate. Unfortunately the unit test may not work on Windows systems since it calls out to the system utility 'dbus-send'.
|
Different crash now with 50ea457, and with --debug to produce some useful output: caught System.NullReferenceException: Object reference not set to an instance of an object 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. |
|
I'm also seeing unit test failures with 50ea457: Runtime Environment - ....................F.F...................................................... Test Case Failures:
at DBus.Tests.IntrospectorTest.InterfaceThroughWireTest () [0x00054] in /home/cmorgan/projects/ikabit/services/motor_lcb/dbus-sharp/tests/IntrospectorTest.cs:95
at DBus.Tests.IntrospectorTest.SimpleInterfaceTest () [0x00031] in /home/cmorgan/projects/ikabit/services/motor_lcb/dbus-sharp/tests/IntrospectorTest.cs:80 |
|
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. |
|
How can I better help debug this? Make a test case or are we doing well On Sun, Feb 22, 2015 at 11:06 AM, Nicholas [email protected] wrote:
|
|
Apply the last two commits and let's see another stack trace. :-) |
|
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] at DBus.DBusMemberTable.GetPropertyAllCall (System.String iface) at DBus.ExportObject.HandlePropertyCall (DBus.Protocol.MessageContainer at DBus.ExportObject.HandleMethodCall (DBus.Protocol.MessageContainer at DBus.Connection.HandleMethodCall (DBus.Protocol.MessageContainer at DBus.Connection.HandleMessage (DBus.Protocol.Message msg) [0x00113] in at DBus.Connection.Iterate () [0x0000f] in at simulator.MainClass.Main (System.String[] args) [0x00225] in Unhandled Exception: System.ArgumentNullException: Argument cannot be null. Parameter name: interface at DBus.TypeImplementer.GenGetAllCall (System.Type interface) [0x00281] at DBus.DBusMemberTable.GetPropertyAllCall (System.String iface) at DBus.ExportObject.HandlePropertyCall (DBus.Protocol.MessageContainer at DBus.ExportObject.HandleMethodCall (DBus.Protocol.MessageContainer at DBus.Connection.HandleMethodCall (DBus.Protocol.MessageContainer at DBus.Connection.HandleMessage (DBus.Protocol.Message msg) [0x00113] in at DBus.Connection.Iterate () [0x0000f] in at simulator.MainClass.Main (System.String[] args) [0x00225] in On Sun, Feb 22, 2015 at 1:04 PM, Nicholas [email protected] wrote:
|
|
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. |
|
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 b09350d which I guess was part of a rebased branch that no longer exists. We started pulling in 3bf6096 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) |
|
"GeFrequency" spelling, prehaps? Or if this is invoked via d-feet, maybe something in the introspection data isn't quite right? |
|
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() |
|
@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. |
|
@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. |
|
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 [email protected] 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? |
|
I'm not sure what you are asking for here. You mean creating an object to On Mon, Feb 23, 2015 at 4:27 PM, Nicholas [email protected] wrote:
|
|
Don't worry, I'll have a good look at the test case on Wednesday night. |
|
Cool. Let me know if I can help out with testing. On Mon, Feb 23, 2015 at 5:40 PM, Nicholas [email protected] wrote:
|
In recent commits I had neglected to consider that some users of dbus-sharp will be subclassing MarshalByRefObject. This commit corrects that, adding a simple GetHierarchy function to return the type and its ancestors.
|
Working much better after 50cee72, a handful of calls I tried are working without issue. Looks like that resolved it. |
|
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... |
|
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 Please let me know if you need more information. Revision DBus.Transports.Transport.ReadMessageReal () in /home/user/product/services/service/dbus-sharp/src/Protocol/Transport.cs:233 This is the definition of object manager public delegate void InterfacesAddedHandler(ObjectPath object_path, public delegate void InterfacesRemovedHandler(ObjectPath object_path, string[] interfaces); [Interface("org.freedesktop.ObjectManager")] |
|
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! |
|
Old and stale, moved to #66. |
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.