Issue #413: Structure leaves always one element in ThreadLocal set

CHANGES.md

@@ -55,6 +55,7 @@ Bug Fixes
 * [#388](https://github.com/twall/jna/issues/388): Ensure native library always opened with provided flags - [@zolyfarkas](https://github.com/zolyfarkas).  
 * [#403](https://github.com/twall/jna/pull/403): Fix `com.sun.jna.platform.win32.COM.COMUtils.SUCCEEDED` and `FAILED` - [@lwahonen](https://github.com/lwahonen).
 * [#404](https://github.com/twall/jna/pull/404): Fix `VARIANT` constructors for `int`, `short`, and `long` - [@lwahonen](https://github.com/lwahonen).
+* [#420](https://github.com/twall/jna/pull/420): Fix structure leaving always one element in ThreadLocal set - [@sjappig](https://github.com/sjappig).
 
 Release 4.1
 ===========

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();
     }

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]);
+    }
 }