Commit f62858a
committed
[Java.Interop.Dynamic] Support field lookup fallback.
System.Dynamic.DynamicMetaObject has a concept of "fallback"
metaobjects: if the DynamicMetaObject can't perform a particular
operation, the "host site" can be called upon to perform the
operation instead. See §5.4.3.1 in:
https://www.codeplex.com/Download?ProjectName=dlr&DownloadId=127512
For example, consider DynamicJavaClass.JniClassName: this is a public
member (which perhaps shouldn't be?) which contains the JNI class name
provided to the constructor:
partial class DynamicJavaClass {
public DynamicJavaClass (string jniClassName);
public string JniClassName {get;}
}
It is thus "obvious" to want to access this member:
var d = new DynamicJavaClass ("foo/Bar");
// d.JniClassName == "foo/Bar"
Accessing such "real" members requires using the fallback mechanism;
without it, attempting to access DynamicJavaClass.JniClassName results
in a runtime exception from JniPeerStaticFields.GetValue()[0].
dynamic d = new DynamicJavaClass ("foo/Bar");
string s = d.JniClassName;
// BOOM [0]
The cause of the crash is a bug (lol): JniTypeInfo.ToString() was
used, not JniTypeInfo.JniTypeReference. The latter always uses `L...;`
for reference types; the former does not, and the latter is required
for JNIEnv::GetStaticFieldID(). Since `java/lang/String` isn't in the
correct format, it threw.
Fixing this results in a different, expected, exception [1]. Because
the Java type doesn't contain a JniClassName field,
JNIEnv::GetStaticFieldID() raises an exception, stopping all progress.
The fix is to take a page out of DynamicObject:
https://msdn.microsoft.com/en-us/library/system.dynamic.dynamicobject(v=vs.110).aspx
https://github.com/Microsoft/referencesource/blob/9da503f9ef21e8d1f2905c78d4e3e5cbb3d6f85a/System.Core/Microsoft/Scripting/Actions/DynamicObject.cs
Instead of immediately performing the operation, rename
DynamicJavaClass.GetStaticFieldValue() to
DynamicJavaClass.TryGetStaticMemberValue(), which (1) returns `bool`,
and (2) uses an `out` parameter to store the value.
DynamicJavaClass.TryGetStaticMemberValue() internally catches the
JavaException, and returns whether or not the lookup was successful.
Consider:
dynamic d = new DynamicJavaClass ("java/lang/Object");
string s = d.JniClassName;
Instead of converting this into:
string s = (string) d.GetStaticFieldValue ("JniClassName", typeof (string));
we instead generate:
object __v;
string s = d.TryGetStaticMemberValue ("JniClassName", typeof (string), out __v)
? (string) __v
: (string) [[ Insert GetMemberBinder.FallbackGetMember() behavior here ]]
which amounts to:
object __v;
string s = d.TryGetStaticMemberValue ("JniClassName", typeof (string), out __v)
? (string) __v
: (string) d.JniClassName;
This works as expected, and allows JNI lookup to be performed first,
and when it fails to fallback on e.g. C# reflection.
There is one flaw, however: we still require a conversion step.
"Explicit" member access works:
dynamic d = new DynamicJavaClass ("java/lang/Object");
string s = d.JniClassName;
// Now works
"Implicit" member access fails:
dynamic d = new DynamicJavaClass ("java/lang/Object");
Assert.AreEqual ("java/lang/Object", d.JniClassName);
// BOOM [2]
The cause for this is that "GetMember" returns an intermediate
DeferredConvert<T> instance instead of the member itself, because we
need to know the final type to perform JNI lookup. When there is no
"convert to the actual type" step, as above, we're left with the
intermediate, and things blow up.
I can't think of a good fix to this, though a "workaround" is to
provide the conversion, e.g. through a cast:
dynamic d = new DynamicJavaClass ("java/lang/Object");
Assert.AreEqual ("java/lang/Object", (string) d.JniClassName);
// Works.
Aside: We change the default BindingRestrictions for
DynamicMetaObject<T> to (hopefully?) better support call-site caching.
I'm still not sure how to even TEST this, much less "properly" fix it
(is it broken?), but I *think* this would be "more correct."
[0]: System.NotSupportedException : Unsupported argument type: java/lang/String
at Java.Interop.JniPeerStaticFields.GetValue (string) [0x0012d] in .../src/Java.Interop/Java.Interop/JniPeerStaticFields.cs:49
at Java.Interop.Dynamic.DynamicJavaClass.GetStaticFieldValue (string,System.Type) [0x00058] in ...k/src/Java.Interop.Dynamic/Java.Interop.Dynamic/DynamicJavaClass.cs:88
at (wrapper dynamic-method) object.CallSite.Target (System.Runtime.CompilerServices.Closure,System.Runtime.CompilerServices.CallSite,object)
at System.Dynamic.UpdateDelegates.UpdateAndExecute1<object, string> (System.Runtime.CompilerServices.CallSite,object)
at Java.Interop.DynamicTests.DynamicJavaClassTests.JniClassName () [0x0000c] in .../src/Java.Interop.Dynamic/Tests/Java.Interop.Dynamic/DynamicJavaClassTests.cs:29
at (wrapper managed-to-native) System.Reflection.MonoMethod.InternalInvoke (System.Reflection.MonoMethod,object,object[],System.Exception&)
at System.Reflection.MonoMethod.Invoke (object,System.Reflection.BindingFlags,System.Reflection.Binder,object[],System.Globalization.CultureInfo) [0x00054] in /private/tmp/source-mono-mac-4.0.0-branch-c5sr4/bockbuild-mono-4.0.0-branch/profiles/mono-mac-xamarin/build-root/mono-4.0.4/mcs/class/corlib/System.Reflection/MonoMethod.cs:230
[1]: Java.Interop.JavaException : JniClassName
at Java.Interop.JniEnvironment/Members.GetStaticFieldID (Java.Interop.JniReferenceSafeHandle,string,string) [0x00086] in .../src/Java.Interop/Java.Interop/JniEnvironment.g.cs:2612
at Java.Interop.JniType.GetStaticField (string,string) [0x0000f] in .../src/Java.Interop/Java.Interop/JniType.cs:212
at Java.Interop.JniPeerStaticFields.GetFieldID (string) [0x0003f] in .../src/Java.Interop/Java.Interop/JniPeerStaticFields.cs:23
at Java.Interop.JniPeerStaticFields.GetObjectValue (string) [0x00003] in .../src/Java.Interop/Java.Interop/JniPeerFields.cs:268
at Java.Interop.JniPeerStaticFields.GetValue (string) [0x000f6] in .../src/Java.Interop/Java.Interop/JniPeerStaticFields.cs:44
at Java.Interop.Dynamic.DynamicJavaClass.GetStaticFieldValue (string,System.Type) [0x00052] in .../src/Java.Interop.Dynamic/Java.Interop.Dynamic/DynamicJavaClass.cs:88
at (wrapper dynamic-method) object.CallSite.Target (System.Runtime.CompilerServices.Closure,System.Runtime.CompilerServices.CallSite,object)
at System.Dynamic.UpdateDelegates.UpdateAndExecute1<object, string> (System.Runtime.CompilerServices.CallSite,object)
at Java.Interop.DynamicTests.DynamicJavaClassTests.JniClassName () [0x0000c] in .../src/Java.Interop.Dynamic/Tests/Java.Interop.Dynamic/DynamicJavaClassTests.cs:29
at (wrapper managed-to-native) System.Reflection.MonoMethod.InternalInvoke (System.Reflection.MonoMethod,object,object[],System.Exception&)
at System.Reflection.MonoMethod.Invoke (object,System.Reflection.BindingFlags,System.Reflection.Binder,object[],System.Globalization.CultureInfo) [0x00054] in /private/tmp/source-mono-mac-4.0.0-branch-c5sr4/bockbuild-mono-4.0.0-branch/profiles/mono-mac-xamarin/build-root/mono-4.0.4/mcs/class/corlib/System.Reflection/MonoMethod.cs:230
--- End of managed Java.Interop.JavaException stack trace ---
java.lang.NoSuchFieldError: JniClassName
[2]: Test Failure : Java.Interop.DynamicTests.DynamicJavaClassTests.JniClassName
Expected: "java/util/Arrays"
But was: <Java.Interop.Dynamic.DeferredConvert`1[Java.Interop.Dynamic.DynamicJavaClass]>1 parent 3b1a91a commit f62858a
File tree
3 files changed
+70
-49
lines changed- src/Java.Interop.Dynamic
- Java.Interop.Dynamic
- Tests/Java.Interop.Dynamic
3 files changed
+70
-49
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
41 | 41 | | |
42 | 42 | | |
43 | 43 | | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
44 | 48 | | |
45 | 49 | | |
Lines changed: 52 additions & 46 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
| 12 | + | |
| 13 | + | |
12 | 14 | | |
13 | 15 | | |
14 | 16 | | |
| |||
75 | 77 | | |
76 | 78 | | |
77 | 79 | | |
78 | | - | |
| 80 | + | |
79 | 81 | | |
80 | 82 | | |
81 | 83 | | |
82 | 84 | | |
83 | | - | |
| 85 | + | |
84 | 86 | | |
85 | 87 | | |
86 | 88 | | |
87 | | - | |
88 | | - | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
89 | 99 | | |
90 | 100 | | |
91 | 101 | | |
92 | 102 | | |
93 | 103 | | |
94 | 104 | | |
95 | | - | |
| 105 | + | |
96 | 106 | | |
97 | 107 | | |
98 | 108 | | |
99 | | - | |
100 | | - | |
101 | | - | |
102 | | - | |
103 | | - | |
104 | 109 | | |
105 | 110 | | |
106 | 111 | | |
| |||
131 | 136 | | |
132 | 137 | | |
133 | 138 | | |
134 | | - | |
| 139 | + | |
135 | 140 | | |
136 | 141 | | |
137 | 142 | | |
138 | 143 | | |
139 | 144 | | |
140 | 145 | | |
| 146 | + | |
| 147 | + | |
141 | 148 | | |
142 | 149 | | |
143 | 150 | | |
| |||
196 | 203 | | |
197 | 204 | | |
198 | 205 | | |
199 | | - | |
200 | | - | |
201 | | - | |
202 | | - | |
203 | | - | |
204 | | - | |
205 | | - | |
206 | | - | |
207 | | - | |
208 | | - | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
209 | 215 | | |
210 | 216 | | |
211 | 217 | | |
| |||
257 | 263 | | |
258 | 264 | | |
259 | 265 | | |
260 | | - | |
| 266 | + | |
261 | 267 | | |
262 | | - | |
263 | | - | |
264 | | - | |
265 | | - | |
266 | | - | |
267 | | - | |
268 | | - | |
269 | | - | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
270 | 272 | | |
271 | | - | |
| 273 | + | |
272 | 274 | | |
273 | | - | |
274 | | - | |
| 275 | + | |
275 | 276 | | |
276 | 277 | | |
277 | 278 | | |
278 | | - | |
| 279 | + | |
279 | 280 | | |
280 | | - | |
| 281 | + | |
281 | 282 | | |
282 | 283 | | |
283 | | - | |
| 284 | + | |
284 | 285 | | |
285 | 286 | | |
286 | 287 | | |
287 | 288 | | |
288 | | - | |
289 | | - | |
290 | | - | |
291 | | - | |
292 | | - | |
293 | | - | |
294 | | - | |
295 | | - | |
296 | | - | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
297 | 303 | | |
298 | | - | |
| 304 | + | |
299 | 305 | | |
300 | 306 | | |
301 | 307 | | |
| |||
Lines changed: 14 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
16 | 16 | | |
17 | 17 | | |
18 | 18 | | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
19 | 22 | | |
20 | 23 | | |
21 | 24 | | |
22 | 25 | | |
23 | 26 | | |
24 | 27 | | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
25 | 36 | | |
26 | 37 | | |
27 | 38 | | |
28 | | - | |
| 39 | + | |
29 | 40 | | |
30 | 41 | | |
31 | 42 | | |
| |||
35 | 46 | | |
36 | 47 | | |
37 | 48 | | |
38 | | - | |
| 49 | + | |
39 | 50 | | |
40 | 51 | | |
41 | 52 | | |
42 | 53 | | |
43 | 54 | | |
44 | 55 | | |
45 | 56 | | |
46 | | - | |
| 57 | + | |
47 | 58 | | |
48 | 59 | | |
49 | 60 | | |
| |||
0 commit comments