diff --git a/bind/java/Seq.java b/bind/java/Seq.java index 959ddd5..7c0b4a8 100644 --- a/bind/java/Seq.java +++ b/bind/java/Seq.java @@ -58,6 +58,7 @@ public class Seq { public native void writeByteArray(byte[] v); public void writeRef(Ref ref) { + tracker.inc(ref); writeInt32(ref.refnum); } @@ -160,20 +161,26 @@ public class Seq { // keyed by the Ref number. When the JVM calls finalize, we ask Go // to clear the entry in the map. public static final class Ref { - // ref < 0: Go object tracked by Java - // ref > 0: Java object tracked by Go + // refnum < 0: Go object tracked by Java + // refnum > 0: Java object tracked by Go int refnum; - public Seq.Object obj; + + int refcnt; // for Java obj: track how many times sent to Go. + + public Seq.Object obj; // for Java obj: pointers to the Java obj. private Ref(int refnum, Seq.Object o) { this.refnum = refnum; + this.refcnt = 0; this.obj = o; - tracker.inc(refnum); } @Override protected void finalize() throws Throwable { - tracker.dec(refnum); + if (refnum < 0) { + // Go object: signal Go to decrement the reference count. + Seq.destroyRef(refnum); + } super.finalize(); } } @@ -188,53 +195,50 @@ public class Seq { // to make debugging by reading Seq hex a little easier. private int next = 42; // next Java object ref - // TODO(crawshaw): We could cut down allocations for frequently - // sent Go objects by maintaining a map to weak references. This - // however, would require allocating two objects per reference - // instead of one. It also introduces weak references, the bane - // of any Java debugging session. - // - // When we have real code, examine the tradeoffs. - - // Number of active references to a Go object. refnum -> count - private SparseIntArray goObjs = new SparseIntArray(); - // Java objects that have been passed to Go. refnum -> Ref // The Ref obj field is non-null. // This map pins Java objects so they don't get GCed while the // only reference to them is held by Go code. private SparseArray javaObjs = new SparseArray(); - // inc increments the reference count to a Go object. - synchronized void inc(int refnum) { - if (refnum > 0) { - return; // we don't count java objects - } - int count = goObjs.get(refnum); - if (count == Integer.MAX_VALUE) { - throw new RuntimeException("refnum " + refnum + " overflow"); - } - goObjs.put(refnum, count+1); - } - - // dec decrements the reference count to a Go object. - // If the count reaches zero, the Go reference tracker is informed. - synchronized void dec(int refnum) { - if (refnum > 0) { - // Java objects are removed on request of Go. - javaObjs.remove(refnum); + // inc increments the reference count of a Java object when it + // is sent to Go. + synchronized void inc(Ref ref) { + int refnum = ref.refnum; + if (refnum <= 0) { + // We don't keep track of the Go object. return; } - int count = goObjs.get(refnum); - if (count == 0) { - throw new RuntimeException("refnum " + refnum + " underflow"); + // Count Java objects passed to Go. + if (ref.refcnt == Integer.MAX_VALUE) { + throw new RuntimeException("refnum " + refnum + " overflow"); } - count--; - if (count <= 0) { - goObjs.delete(refnum); - Seq.destroyRef(refnum); - } else { - goObjs.put(refnum, count); + ref.refcnt++; + Ref obj = javaObjs.get(refnum); + if (obj == null) { + javaObjs.put(refnum, ref); + } + } + + // dec decrements the reference count of a Java object when + // Go signals a corresponding proxy object is finalized. + // If the count reaches zero, the Java object is removed + // from the javaObjs map. + synchronized void dec(int refnum) { + if (refnum <= 0) { + // We don't keep track of the Go object. + // This must not happen. + Log.wtf("Seq", "dec request for Go object "+ refnum); + return; + } + // Java objects are removed on request of Go. + Ref obj = javaObjs.get(refnum); + if (obj == null) { + throw new RuntimeException("referenced Java object is not found: refnum="+refnum); + } + obj.refcnt--; + if (obj.refcnt <= 0) { + javaObjs.remove(refnum); } } @@ -251,6 +255,14 @@ public class Seq { // get returns an existing Ref to either a Java or Go object. // It may be the first time we have seen the Go object. + // + // TODO(crawshaw): We could cut down allocations for frequently + // sent Go objects by maintaining a map to weak references. This + // however, would require allocating two objects per reference + // instead of one. It also introduces weak references, the bane + // of any Java debugging session. + // + // When we have real code, examine the tradeoffs. synchronized Ref get(int refnum) { if (refnum > 0) { Ref ref = javaObjs.get(refnum); @@ -258,8 +270,10 @@ public class Seq { throw new RuntimeException("unknown java Ref: "+refnum); } return ref; + } else { + // Go object. + return new Ref(refnum, null); } - return new Ref(refnum, null); } } } diff --git a/bind/java/SeqTest.java b/bind/java/SeqTest.java index 1ed7fae..27bef2b 100644 --- a/bind/java/SeqTest.java +++ b/bind/java/SeqTest.java @@ -4,6 +4,7 @@ package go; +import android.util.Log; import android.test.suitebuilder.annotation.Suppress; import android.test.AndroidTestCase; import android.test.MoreAsserts; @@ -122,8 +123,6 @@ public class SeqTest extends AndroidTestCase { assertEquals("S should be collected", 1, collected); } - boolean finalizedAnI; - private class AnI extends Testpkg.I.Stub { public void E() throws Exception { throw new Exception("my exception from E"); @@ -160,12 +159,8 @@ public class SeqTest extends AndroidTestCase { return name; } - @Override - public void finalize() throws Throwable { - finalizedAnI = true; - super.finalize(); - } } + // TODO(hyangah): add tests for methods that take parameters. public void testInterfaceMethodReturnsError() { @@ -189,11 +184,19 @@ public class SeqTest extends AndroidTestCase { obj.name = "testing AnI.I"; Testpkg.I i = Testpkg.CallI(obj); assertEquals("Want AnI.I to return itself", i.String(), obj.String()); + + runGC(); + + i = Testpkg.CallI(obj); + assertEquals("Want AnI.I to return itself", i.String(), obj.String()); } public void testInterfaceMethodReturnsStructPointer() { final AnI obj = new AnI(); - Testpkg.S s = Testpkg.CallS(obj); + for (int i = 0; i < 5; i++) { + Testpkg.S s = Testpkg.CallS(obj); + runGC(); + } } public void testInterfaceMethodTakesStructPointer() { @@ -219,14 +222,22 @@ public class SeqTest extends AndroidTestCase { } } - /* Suppress this test for now; it's flaky or broken. */ - @Suppress + boolean finalizedAnI; + + private class AnI_Traced extends AnI { + @Override + public void finalize() throws Throwable { + finalizedAnI = true; + super.finalize(); + } + } + public void testJavaRefGC() { finalizedAnI = false; - AnI obj = new AnI(); - runGC(); + AnI obj = new AnI_Traced(); Testpkg.CallF(obj); assertTrue("want F to be called", obj.calledF); + Testpkg.CallF(obj); obj = null; runGC(); assertTrue("want obj to be collected", finalizedAnI); @@ -234,7 +245,15 @@ public class SeqTest extends AndroidTestCase { public void testJavaRefKeep() { finalizedAnI = false; - AnI obj = new AnI(); + AnI obj = new AnI_Traced(); + Testpkg.CallF(obj); + Testpkg.CallF(obj); + obj = null; + runGC(); + assertTrue("want obj not to be kept by Go", finalizedAnI); + + finalizedAnI = false; + obj = new AnI_Traced(); Testpkg.Keep(obj); obj = null; runGC(); diff --git a/bind/java/testpkg/testpkg.go b/bind/java/testpkg/testpkg.go index ea291b9..51313bb 100644 --- a/bind/java/testpkg/testpkg.go +++ b/bind/java/testpkg/testpkg.go @@ -76,7 +76,7 @@ func (s *S) String() string { return s.name } -func finalizeInner(*int) { +func finalizeInner(a *int) { numSCollected++ } @@ -144,3 +144,15 @@ func (a *Node) String() string { } return a.V + ":" + a.Next.String() } + +type Receiver interface { + Hello(message string) +} + +func Hello(r Receiver, name string) { + r.Hello(fmt.Sprintf("Hello, %s!\n", name)) +} + +func GarbageCollect() { + runtime.GC() +} diff --git a/bind/seq/buffer.go b/bind/seq/buffer.go index 372ecf2..45f40ca 100644 --- a/bind/seq/buffer.go +++ b/bind/seq/buffer.go @@ -244,14 +244,17 @@ func (b *Buffer) WriteString(v string) { func (b *Buffer) WriteGoRef(obj interface{}) { refs.Lock() num := refs.refs[obj] - if num == 0 { + if num != 0 { + s := refs.objs[num] + refs.objs[num] = countedObj{s.obj, s.cnt + 1} + } else { num = refs.next refs.next-- if refs.next > 0 { panic("refs.next underflow") } refs.refs[obj] = num - refs.objs[num] = obj + refs.objs[num] = countedObj{obj, 1} } refs.Unlock() diff --git a/bind/seq/ref.go b/bind/seq/ref.go index 0ebd896..2eed868 100644 --- a/bind/seq/ref.go +++ b/bind/seq/ref.go @@ -14,19 +14,24 @@ import ( "sync" ) +type countedObj struct { + obj interface{} + cnt int32 +} + // refs stores Go objects that have been passed to another language. var refs struct { sync.Mutex next int32 // next reference number to use for Go object, always negative refs map[interface{}]int32 - objs map[int32]interface{} + objs map[int32]countedObj } func init() { refs.Lock() refs.next = -24 // Go objects get negative reference numbers. Arbitrary starting point. refs.refs = make(map[interface{}]int32) - refs.objs = make(map[int32]interface{}) + refs.objs = make(map[int32]countedObj) refs.Unlock() } @@ -39,22 +44,27 @@ type Ref struct { // Get returns the underlying object. func (r *Ref) Get() interface{} { refs.Lock() - obj, ok := refs.objs[r.Num] + o, ok := refs.objs[r.Num] refs.Unlock() if !ok { panic(fmt.Sprintf("unknown ref %d", r.Num)) } - return obj + return o.obj } -// Delete remove the reference to the underlying object. +// Delete decrements the reference count and removes the pinned object +// from the object map when the reference count becomes zero. func Delete(num int32) { refs.Lock() - obj, ok := refs.objs[num] + defer refs.Unlock() + o, ok := refs.objs[num] if !ok { panic(fmt.Sprintf("seq.Delete unknown refnum: %d", num)) } - delete(refs.objs, num) - delete(refs.refs, obj) - refs.Unlock() + if o.cnt <= 1 { + delete(refs.objs, num) + delete(refs.refs, o.obj) + } else { + refs.objs[num] = countedObj{o.obj, o.cnt - 1} + } }