From bd3f1275aff54e16a132d28efc97396bfd09779d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jari-Pekka=20Ryyn=C3=A4nen?= Date: Wed, 8 Apr 2015 14:44:04 +0300 Subject: [PATCH] Issue #413: Structure leaves always one element in ThreadLocal set --- src/com/sun/jna/Structure.java | 121 +++++++++++++++------------- test/com/sun/jna/StructureTest.java | 22 +++++ 2 files changed, 85 insertions(+), 58 deletions(-) diff --git a/src/com/sun/jna/Structure.java b/src/com/sun/jna/Structure.java index 1e86703ffc..aec06fcff9 100644 --- a/src/com/sun/jna/Structure.java +++ b/src/com/sun/jna/Structure.java @@ -427,72 +427,77 @@ protected synchronized Object initialValue() { // Keep track of what is currently being read/written to avoid redundant // reads (avoids problems with circular references). private static final ThreadLocal busy = new ThreadLocal() { - /** Avoid using a hash-based implementation since the hash code + protected synchronized Object initialValue() { + return new StructureSet(); + } + }; + + /** Avoid using a hash-based implementation since the hash code for a Structure is not immutable. - */ - class StructureSet extends AbstractCollection implements Set { - private Structure[] elements; - private int count; - private void ensureCapacity(int size) { - if (elements == null) { - elements = new Structure[size*3/2]; - } - else if (elements.length < size) { - Structure[] e = new Structure[size*3/2]; - System.arraycopy(elements, 0, e, 0, elements.length); - elements = e; - } - } - public int size() { return count; } - public boolean contains(Object o) { - return indexOf(o) != -1; + */ + static class StructureSet extends AbstractCollection implements Set { + Structure[] elements; + private int count; + private void ensureCapacity(int size) { + if (elements == null) { + elements = new Structure[size*3/2]; + } + else if (elements.length < size) { + Structure[] e = new Structure[size*3/2]; + System.arraycopy(elements, 0, e, 0, elements.length); + elements = e; + } + } + public Structure[] getElements() { + return elements; + } + public int size() { return count; } + public boolean contains(Object o) { + return indexOf(o) != -1; + } + public boolean add(Object o) { + if (!contains(o)) { + ensureCapacity(count+1); + elements[count++] = (Structure)o; } - public boolean add(Object o) { - if (!contains(o)) { - ensureCapacity(count+1); - elements[count++] = (Structure)o; + return true; + } + private int indexOf(Object o) { + Structure s1 = (Structure)o; + for (int i=0;i < count;i++) { + Structure s2 = elements[i]; + if (s1 == s2 + || (s1.getClass() == s2.getClass() + && s1.size() == s2.size() + && s1.getPointer().equals(s2.getPointer()))) { + return i; } - return true; } - private int indexOf(Object o) { - Structure s1 = (Structure)o; - for (int i=0;i < count;i++) { - Structure s2 = elements[i]; - if (s1 == s2 - || (s1.getClass() == s2.getClass() - && s1.size() == s2.size() - && s1.getPointer().equals(s2.getPointer()))) { - return i; - } - } - return -1; - } - public boolean remove(Object o) { - int idx = indexOf(o); - if (idx != -1) { - if (--count > 0) { - elements[idx] = elements[count]; - elements[count] = null; - } - return true; - } - return false; - } - /** Simple implementation so that toString() doesn't break. - Provides an iterator over a snapshot of this Set. - */ - public Iterator iterator() { - Structure[] e = new Structure[count]; - if (count > 0) { - System.arraycopy(elements, 0, e, 0, count); + return -1; + } + public boolean remove(Object o) { + int idx = indexOf(o); + if (idx != -1) { + if (--count >= 0) { + elements[idx] = elements[count]; + elements[count] = null; } - return Arrays.asList(e).iterator(); + return true; } + return false; } - protected synchronized Object initialValue() { - return new StructureSet(); + /** Simple implementation so that toString() doesn't break. + Provides an iterator over a snapshot of this Set. + */ + public Iterator iterator() { + Structure[] e = new Structure[count]; + if (count > 0) { + System.arraycopy(elements, 0, e, 0, count); + } + return Arrays.asList(e).iterator(); } - }; + } + static Set busy() { return (Set)busy.get(); } diff --git a/test/com/sun/jna/StructureTest.java b/test/com/sun/jna/StructureTest.java index 0699a66bb3..eec0296a4d 100644 --- a/test/com/sun/jna/StructureTest.java +++ b/test/com/sun/jna/StructureTest.java @@ -16,9 +16,11 @@ import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.Set; import junit.framework.TestCase; +import com.sun.jna.Structure.StructureSet; import com.sun.jna.ptr.ByteByReference; import com.sun.jna.ptr.IntByReference; import com.sun.jna.ptr.LongByReference; @@ -1968,4 +1970,24 @@ protected List getFieldOrder() { s.read(); assertEquals("String not decoded properly on read", VALUE, s.field); } + + public void testThreadLocalSetReleasesReferences() { + class TestStructure extends Structure { + public String field; + protected List getFieldOrder() { + return Arrays.asList(new String[] { "field" }); + } + } + + TestStructure ts1 = new TestStructure(); + TestStructure ts2 = new TestStructure(); + + StructureSet structureSet = (StructureSet) Structure.busy(); + structureSet.add(ts1); + structureSet.add(ts2); + structureSet.remove(ts1); + assertNotNull(structureSet.elements[0]); + structureSet.remove(ts2); + assertNull(structureSet.elements[0]); + } }