Skip to content

Commit

Permalink
[Java.Interop] Prevent premature collection w/ JniInstance* (#768)
Browse files Browse the repository at this point in the history 
Context: dotnet/android@e88cfbc

While writing the commit message for xamarin/xamarin-android/@e88cfbcf,
it occurred to me that the same fundamental scenario of:

	CallIntoJava (new JavaLangObjectSubclass ().Handle);
	// GC collects instance after `.Handle`, before `CallIntoJava()`

could apply to `JniPeerMembers.JniInstanceMethods.Invoke*()`
invocations:

	JniArgumentValue* __args = …;
	_members.InstanceMethods.InvokeVirtualObjectMethod (__id, this, __args);
	// What prevents `this` from being collected "too soon"?

Address this: update `JniPeerMembers.JniInstanceMethods.Invoke*()`
so that there is a `GC.KeepAlive(self)` after accessing
`self.PeerReference`.  This will ensure that `self` isn't collected
"during" `JniEnvironment.InstanceMethods.Call*Method()` invocations.

Likewise update `JniPeerMembers.JniInstanceFields.Get*Value()` and
`JniPeerMembers.JniInstanceFields.Set*Value()` so that there is a
`GC.KeepAlive(self)` after the `JniEnvironment.InstanceFields.*`
invocation.
  • Loading branch information
@jonpryor
jonpryor committed Jan 5, 2021
1 parent 79d9533 commit 04c80fc
Show file tree
Hide file tree
Showing 4 changed files with 168 additions and 90 deletions.
47 changes: 38 additions & 9 deletions src/Java.Interop/Java.Interop/JniPeerMembers.JniFields.cs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#nullable enable

using System;

namespace Java.Interop {

partial class JniPeerMembers {
Expand All @@ -12,7 +14,9 @@ public bool GetBooleanValue (
JniPeerMembers.AssertSelf (self);

var f = GetFieldInfo (encodedMember);
return JniEnvironment.InstanceFields.GetBooleanField (self.PeerReference, f);
var r = JniEnvironment.InstanceFields.GetBooleanField (self.PeerReference, f);
GC.KeepAlive (self);
return r;
}

public void SetValue (string encodedMember, IJavaPeerable self, bool value)
Expand All @@ -21,6 +25,7 @@ public void SetValue (string encodedMember, IJavaPeerable self, bool value)

var f = GetFieldInfo (encodedMember);
JniEnvironment.InstanceFields.SetBooleanField (self.PeerReference, f, value);
GC.KeepAlive (self);
}

public sbyte GetSByteValue (
Expand All @@ -30,7 +35,9 @@ public sbyte GetSByteValue (
JniPeerMembers.AssertSelf (self);

var f = GetFieldInfo (encodedMember);
return JniEnvironment.InstanceFields.GetByteField (self.PeerReference, f);
var r = JniEnvironment.InstanceFields.GetByteField (self.PeerReference, f);
GC.KeepAlive (self);
return r;
}

public void SetValue (string encodedMember, IJavaPeerable self, sbyte value)
Expand All @@ -39,6 +46,7 @@ public void SetValue (string encodedMember, IJavaPeerable self, sbyte value)

var f = GetFieldInfo (encodedMember);
JniEnvironment.InstanceFields.SetByteField (self.PeerReference, f, value);
GC.KeepAlive (self);
}

public char GetCharValue (
Expand All @@ -48,7 +56,9 @@ public char GetCharValue (
JniPeerMembers.AssertSelf (self);

var f = GetFieldInfo (encodedMember);
return JniEnvironment.InstanceFields.GetCharField (self.PeerReference, f);
var r = JniEnvironment.InstanceFields.GetCharField (self.PeerReference, f);
GC.KeepAlive (self);
return r;
}

public void SetValue (string encodedMember, IJavaPeerable self, char value)
Expand All @@ -57,6 +67,7 @@ public void SetValue (string encodedMember, IJavaPeerable self, char value)

var f = GetFieldInfo (encodedMember);
JniEnvironment.InstanceFields.SetCharField (self.PeerReference, f, value);
GC.KeepAlive (self);
}

public short GetInt16Value (
Expand All @@ -66,7 +77,9 @@ public short GetInt16Value (
JniPeerMembers.AssertSelf (self);

var f = GetFieldInfo (encodedMember);
return JniEnvironment.InstanceFields.GetShortField (self.PeerReference, f);
var r = JniEnvironment.InstanceFields.GetShortField (self.PeerReference, f);
GC.KeepAlive (self);
return r;
}

public void SetValue (string encodedMember, IJavaPeerable self, short value)
Expand All @@ -75,6 +88,7 @@ public void SetValue (string encodedMember, IJavaPeerable self, short value)

var f = GetFieldInfo (encodedMember);
JniEnvironment.InstanceFields.SetShortField (self.PeerReference, f, value);
GC.KeepAlive (self);
}

public int GetInt32Value (
Expand All @@ -84,7 +98,9 @@ public int GetInt32Value (
JniPeerMembers.AssertSelf (self);

var f = GetFieldInfo (encodedMember);
return JniEnvironment.InstanceFields.GetIntField (self.PeerReference, f);
var r = JniEnvironment.InstanceFields.GetIntField (self.PeerReference, f);
GC.KeepAlive (self);
return r;
}

public void SetValue (string encodedMember, IJavaPeerable self, int value)
Expand All @@ -93,6 +109,7 @@ public void SetValue (string encodedMember, IJavaPeerable self, int value)

var f = GetFieldInfo (encodedMember);
JniEnvironment.InstanceFields.SetIntField (self.PeerReference, f, value);
GC.KeepAlive (self);
}

public long GetInt64Value (
Expand All @@ -102,7 +119,9 @@ public long GetInt64Value (
JniPeerMembers.AssertSelf (self);

var f = GetFieldInfo (encodedMember);
return JniEnvironment.InstanceFields.GetLongField (self.PeerReference, f);
var r = JniEnvironment.InstanceFields.GetLongField (self.PeerReference, f);
GC.KeepAlive (self);
return r;
}

public void SetValue (string encodedMember, IJavaPeerable self, long value)
Expand All @@ -111,6 +130,7 @@ public void SetValue (string encodedMember, IJavaPeerable self, long value)

var f = GetFieldInfo (encodedMember);
JniEnvironment.InstanceFields.SetLongField (self.PeerReference, f, value);
GC.KeepAlive (self);
}

public float GetSingleValue (
Expand All @@ -120,7 +140,9 @@ public float GetSingleValue (
JniPeerMembers.AssertSelf (self);

var f = GetFieldInfo (encodedMember);
return JniEnvironment.InstanceFields.GetFloatField (self.PeerReference, f);
var r = JniEnvironment.InstanceFields.GetFloatField (self.PeerReference, f);
GC.KeepAlive (self);
return r;
}

public void SetValue (string encodedMember, IJavaPeerable self, float value)
Expand All @@ -129,6 +151,7 @@ public void SetValue (string encodedMember, IJavaPeerable self, float value)

var f = GetFieldInfo (encodedMember);
JniEnvironment.InstanceFields.SetFloatField (self.PeerReference, f, value);
GC.KeepAlive (self);
}

public double GetDoubleValue (
Expand All @@ -138,7 +161,9 @@ public double GetDoubleValue (
JniPeerMembers.AssertSelf (self);

var f = GetFieldInfo (encodedMember);
return JniEnvironment.InstanceFields.GetDoubleField (self.PeerReference, f);
var r = JniEnvironment.InstanceFields.GetDoubleField (self.PeerReference, f);
GC.KeepAlive (self);
return r;
}

public void SetValue (string encodedMember, IJavaPeerable self, double value)
Expand All @@ -147,6 +172,7 @@ public void SetValue (string encodedMember, IJavaPeerable self, double value)

var f = GetFieldInfo (encodedMember);
JniEnvironment.InstanceFields.SetDoubleField (self.PeerReference, f, value);
GC.KeepAlive (self);
}

public JniObjectReference GetObjectValue (
Expand All @@ -156,7 +182,9 @@ public JniObjectReference GetObjectValue (
JniPeerMembers.AssertSelf (self);

var f = GetFieldInfo (encodedMember);
return JniEnvironment.InstanceFields.GetObjectField (self.PeerReference, f);
var r = JniEnvironment.InstanceFields.GetObjectField (self.PeerReference, f);
GC.KeepAlive (self);
return r;
}

public void SetValue (string encodedMember, IJavaPeerable self, JniObjectReference value)
Expand All @@ -165,6 +193,7 @@ public void SetValue (string encodedMember, IJavaPeerable self, JniObjectReferen

var f = GetFieldInfo (encodedMember);
JniEnvironment.InstanceFields.SetObjectField (self.PeerReference, f, value);
GC.KeepAlive (self);
}
}

Expand Down
7 changes: 6 additions & 1 deletion src/Java.Interop/Java.Interop/JniPeerMembers.JniFields.tt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
#>
#nullable enable

using System;

namespace Java.Interop {

partial class JniPeerMembers {
Expand All @@ -34,7 +36,9 @@ namespace Java.Interop {
JniPeerMembers.AssertSelf (self);

var f = GetFieldInfo (encodedMember);
return JniEnvironment.InstanceFields.Get<#= info.JniCallType #>Field (self.PeerReference, f);
var r = JniEnvironment.InstanceFields.Get<#= info.JniCallType #>Field (self.PeerReference, f);
GC.KeepAlive (self);
return r;
}

public void SetValue (string encodedMember, IJavaPeerable self, <#= info.ParameterType #> value)
Expand All @@ -43,6 +47,7 @@ namespace Java.Interop {

var f = GetFieldInfo (encodedMember);
JniEnvironment.InstanceFields.Set<#= info.JniCallType #>Field (self.PeerReference, f, value);
GC.KeepAlive (self);
}
<#
}
Expand Down
Loading

0 comments on commit 04c80fc

Please sign in to comment.