Change GstIterator support to stop keeping parent reference alive until GC.

neilcsmith-net · neilcsmith-net · commit 1ff6ce789c2c · 2021-01-07T15:14:22.000Z
Migrate lowlevel GstIterator API to typed pointer.
Move GstIterator from NativeObject subclass to utility method holder.
Directly fill list and free gstiterator eagerly.
Update Element (pads) and Bin (elements) methods.
diff --git a/src/org/freedesktop/gstreamer/Bin.java b/src/org/freedesktop/gstreamer/Bin.java
@@ -1,5 +1,5 @@
 /* 
- * Copyright (c) 2019 Neil C Smith
+ * Copyright (c) 2021 Neil C Smith
  * Copyright (c) 2016 Christophe Lafolet
  * Copyright (c) 2009 Levente Farkas
  * Copyright (C) 2007 Wayne Meissner
@@ -24,12 +24,12 @@
 
 import static org.freedesktop.gstreamer.lowlevel.GstBinAPI.GSTBIN_API;
 
-import com.sun.jna.Pointer;
 import java.util.EnumSet;
 import java.util.List;
 import org.freedesktop.gstreamer.glib.NativeFlags;
 import org.freedesktop.gstreamer.glib.Natives;
 import org.freedesktop.gstreamer.lowlevel.GstAPI.GstCallback;
+import org.freedesktop.gstreamer.lowlevel.GstIteratorPtr;
 import org.freedesktop.gstreamer.lowlevel.GstObjectPtr;
 
 /**
@@ -145,8 +145,8 @@ public void removeMany(Element... elements) {
         GSTBIN_API.gst_bin_remove_many(this, elements);
     }
 
-    private List<Element> elementList(Pointer iter) {
-        return new GstIterator<Element>(iter, Element.class).asList();
+    private List<Element> elementList(GstIteratorPtr iter) {
+        return GstIterator.asList(iter, Element.class);
     }
 
     /**
diff --git a/src/org/freedesktop/gstreamer/Element.java b/src/org/freedesktop/gstreamer/Element.java
@@ -31,10 +31,11 @@
 
 import org.freedesktop.gstreamer.lowlevel.GstAPI.GstCallback;
 import org.freedesktop.gstreamer.lowlevel.GstContextPtr;
+import org.freedesktop.gstreamer.lowlevel.GstIteratorPtr;
+import org.freedesktop.gstreamer.lowlevel.GstObjectPtr;
 
 import static org.freedesktop.gstreamer.lowlevel.GstElementAPI.GSTELEMENT_API;
 import static org.freedesktop.gstreamer.lowlevel.GObjectAPI.GOBJECT_API;
-import org.freedesktop.gstreamer.lowlevel.GstObjectPtr;
 
 /**
  * Abstract base class for all pipeline elements.
@@ -82,7 +83,7 @@ public class Element extends GstObject {
     protected Element(Initializer init) {
         super(init);
     }
-    
+
     Element(Handle handle, boolean needRef) {
         super(handle, needRef);
     }
@@ -346,7 +347,7 @@ public Pad getStaticPad(String padname) {
      * @return the List of {@link Pad}s.
      */
     public List<Pad> getPads() {
-        return new GstIterator<Pad>(GSTELEMENT_API.gst_element_iterate_pads(this), Pad.class).asList();
+        return padList(GSTELEMENT_API.gst_element_iterate_pads(this));
     }
 
     /**
@@ -355,7 +356,7 @@ public List<Pad> getPads() {
      * @return the List of {@link Pad}s.
      */
     public List<Pad> getSrcPads() {
-        return new GstIterator<Pad>(GSTELEMENT_API.gst_element_iterate_src_pads(this), Pad.class).asList();
+        return padList(GSTELEMENT_API.gst_element_iterate_src_pads(this));
     }
 
     /**
@@ -364,7 +365,11 @@ public List<Pad> getSrcPads() {
      * @return the List of {@link Pad}s.
      */
     public List<Pad> getSinkPads() {
-        return new GstIterator<Pad>(GSTELEMENT_API.gst_element_iterate_sink_pads(this), Pad.class).asList();
+        return padList(GSTELEMENT_API.gst_element_iterate_sink_pads(this));
+    }
+    
+    private List<Pad> padList(GstIteratorPtr iter) {
+        return GstIterator.asList(iter, Pad.class);
     }
 
     /**
@@ -770,7 +775,7 @@ public void setContext(Context context) {
 
     /**
      * Gets the context with the context_type set on the element or NULL.
-     * 
+     *
      * @param context_type
      * @return a context or NULL
      */
@@ -780,11 +785,11 @@ public Context getContext(String context_type) {
     }
 
     static class Handle extends GstObject.Handle {
-        
+
         public Handle(GstObjectPtr ptr, boolean ownsHandle) {
             super(ptr, ownsHandle);
         }
-        
+
     }
 
 }
diff --git a/src/org/freedesktop/gstreamer/GstIterator.java b/src/org/freedesktop/gstreamer/GstIterator.java
@@ -1,7 +1,5 @@
 /* 
- * Copyright (c) 2019 Neil C Smith
- * Copyright (c) 2009 Levente Farkas
- * Copyright (c) 2007 Wayne Meissner
+ * Copyright (c) 2021 Neil C Smith
  * 
  * This file is part of gstreamer-java.
  *
@@ -19,96 +17,32 @@
  */
 package org.freedesktop.gstreamer;
 
-import com.sun.jna.Pointer;
-
-import java.util.Collections;
-import java.util.Iterator;
-import java.util.LinkedList;
+import java.util.ArrayList;
 import java.util.List;
 
+import org.freedesktop.gstreamer.glib.NativeObject;
+import org.freedesktop.gstreamer.lowlevel.GstIteratorPtr;
+import org.freedesktop.gstreamer.lowlevel.GstTypes;
 import org.freedesktop.gstreamer.lowlevel.GType;
 import org.freedesktop.gstreamer.lowlevel.GValueAPI;
-import org.freedesktop.gstreamer.lowlevel.GstTypes;
-import org.freedesktop.gstreamer.glib.NativeObject;
-import org.freedesktop.gstreamer.lowlevel.GPointer;
 
 import static org.freedesktop.gstreamer.lowlevel.GstIteratorAPI.GSTITERATOR_API;
 
 /**
- *
+ * Utility class for working with gstiterator.
  */
-class GstIterator<T extends NativeObject> extends NativeObject implements java.lang.Iterable<T> {
-
-    private final GType gtype;
-
-    GstIterator(Pointer ptr, Class<T> cls) {
-        super(new Handle(new GPointer(ptr), true));
-        gtype = GstTypes.typeFor(cls);
-    }
-
-    @Override
-    public Iterator<T> iterator() {
-        return new IteratorImpl();
-    }
-
-    public List<T> asList() {
-        List<T> list = new LinkedList<T>();
-        for (T t : this) {
-            list.add(t);
-        }
-        return Collections.unmodifiableList(list);
-    }
-
-    class IteratorImpl implements java.util.Iterator<T> {
-
-        final GValueAPI.GValue gValue;
-
-        T next;
-
-        IteratorImpl() {
-            gValue = new GValueAPI.GValue(gtype);
-            next = getNext();
-        }
-
-        private T getNext() {
-            if (GSTITERATOR_API.gst_iterator_next(getRawPointer(), gValue) == 1) {
-                T result = (T) gValue.getValue();
-                // reset cached structure or we get a memory leak
-                gValue.reset();
-                return result;
-            } else {
-                return null;
-            }
-        }
-
-        @Override
-        public boolean hasNext() {
-            return next != null;
-        }
-
-        @Override
-        public T next() {
-            T result = next;
-            next = getNext();
-            return result;
-        }
-
-        @Override
-        public void remove() {
-            throw new UnsupportedOperationException("Items cannot be removed.");
-        }
-    }
+class GstIterator {
     
-    private static final class Handle extends NativeObject.Handle {
-
-        public Handle(GPointer ptr, boolean ownsHandle) {
-            super(ptr, ownsHandle);
-        }
-
-        @Override
-        protected void disposeNativeHandle(GPointer ptr) {
-            GSTITERATOR_API.gst_iterator_free(ptr.getPointer());
-        }
-        
+    static <T extends NativeObject> List<T> asList(GstIteratorPtr iter, Class<T> type) {
+        final GType gtype = GstTypes.typeFor(type);
+        final GValueAPI.GValue gValue = new GValueAPI.GValue(gtype);
+        List<T> list = new ArrayList<>();
+        while (GSTITERATOR_API.gst_iterator_next(iter, gValue) == 1) {
+            list.add((T) gValue.getValue());
+        }
+        gValue.reset();
+        GSTITERATOR_API.gst_iterator_free(iter);
+        return list;
     }
+    
 }
diff --git a/src/org/freedesktop/gstreamer/lowlevel/GstBinAPI.java b/src/org/freedesktop/gstreamer/lowlevel/GstBinAPI.java
@@ -1,4 +1,5 @@
 /* 
+ * Copyright (c) 2021 Neil C Smith
  * Copyright (c) 2009 Levente Farkas
  * Copyright (c) 2007, 2008 Wayne Meissner
  * 
@@ -43,12 +44,12 @@ public interface GstBinAPI extends com.sun.jna.Library {
     @CallerOwnsReturn Element gst_bin_get_by_name(Bin bin, String name);
     @CallerOwnsReturn Element gst_bin_get_by_name_recurse_up(Bin bin, String name);
     @CallerOwnsReturn Element gst_bin_get_by_interface(Bin bin, GType iface);
-    Pointer gst_bin_iterate_elements(Bin bin);
-    Pointer gst_bin_iterate_sorted(Bin bin);
-    Pointer gst_bin_iterate_recurse(Bin bin);
-    Pointer gst_bin_iterate_sinks(Bin bin);
-    Pointer gst_bin_iterate_sources(Bin bin);
-    Pointer gst_bin_iterate_all_by_interface(Bin bin, GType iface);
+    GstIteratorPtr gst_bin_iterate_elements(Bin bin);
+    GstIteratorPtr gst_bin_iterate_sorted(Bin bin);
+    GstIteratorPtr gst_bin_iterate_recurse(Bin bin);
+    GstIteratorPtr gst_bin_iterate_sinks(Bin bin);
+    GstIteratorPtr gst_bin_iterate_sources(Bin bin);
+    GstIteratorPtr gst_bin_iterate_all_by_interface(Bin bin, GType iface);
 
     //Debugging
     void gst_debug_bin_to_dot_file (Bin bin, int details, String file_name);
diff --git a/src/org/freedesktop/gstreamer/lowlevel/GstElementAPI.java b/src/org/freedesktop/gstreamer/lowlevel/GstElementAPI.java
@@ -1,4 +1,5 @@
 /* 
+ * Copyright (c) 2021 Neil C Smith
  * Copyright (c) 2009 Levente Farkas
  * Copyright (c) 2007, 2008 Wayne Meissner
  * 
@@ -83,9 +84,9 @@ boolean gst_element_seek(Element elem, double rate, Format format, int flags,
     boolean gst_element_link_pads_filtered(Element src, String srcpadname, Element dest, String destpadname,
             Caps filter);
     
-    Pointer gst_element_iterate_pads(Element element);
-    Pointer gst_element_iterate_src_pads(Element element);
-    Pointer gst_element_iterate_sink_pads(Element element);
+    GstIteratorPtr gst_element_iterate_pads(Element element);
+    GstIteratorPtr gst_element_iterate_src_pads(Element element);
+    GstIteratorPtr gst_element_iterate_sink_pads(Element element);
     /* factory management */
     ElementFactory gst_element_get_factory(Element element);
     @CallerOwnsReturn Bus gst_element_get_bus(Element element);
diff --git a/src/org/freedesktop/gstreamer/lowlevel/GstIteratorAPI.java b/src/org/freedesktop/gstreamer/lowlevel/GstIteratorAPI.java
@@ -1,7 +1,5 @@
 /* 
- * Copyright (c) 2016 Neil C Smith
- * Copyright (c) 2009 Levente Farkas
- * Copyright (c) 2007, 2008 Wayne Meissner
+ * Copyright (c) 2021 Neil C Smith
  * 
  * This file is part of gstreamer-java.
  *
@@ -20,15 +18,14 @@
 
 package org.freedesktop.gstreamer.lowlevel;
 
-import com.sun.jna.Pointer;
-
 /**
  * GstIterator functions
  */
 public interface GstIteratorAPI extends com.sun.jna.Library {
+    
     GstIteratorAPI GSTITERATOR_API = GstNative.load(GstIteratorAPI.class);
 
-    void gst_iterator_free(Pointer iter);
-    int gst_iterator_next(Pointer iter, GValueAPI.GValue next);
-    void gst_iterator_resync(Pointer iter);
+    void gst_iterator_free(GstIteratorPtr iter);
+    int gst_iterator_next(GstIteratorPtr iter, GValueAPI.GValue next);
+    void gst_iterator_resync(GstIteratorPtr iter);
 }
diff --git a/src/org/freedesktop/gstreamer/lowlevel/GstIteratorPtr.java b/src/org/freedesktop/gstreamer/lowlevel/GstIteratorPtr.java
@@ -0,0 +1,32 @@
+/*
+ * Copyright (c) 2021 Neil C Smith
+ * 
+ * This file is part of gstreamer-java.
+ *
+ * This code is free software: you can redistribute it and/or modify it under the terms of the GNU
+ * Lesser General Public License version 3 only, as published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without
+ * even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License version 3 for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License version 3 along with
+ * this work. If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.freedesktop.gstreamer.lowlevel;
+
+import com.sun.jna.Pointer;
+
+/**
+ * GstIterator pointer.
+ */
+public class GstIteratorPtr extends GPointer {
+
+    public GstIteratorPtr() {
+    }
+
+    public GstIteratorPtr(Pointer ptr) {
+        super(ptr);
+    }
+
+}
diff --git a/test/org/freedesktop/gstreamer/GarbageCollectionTest.java b/test/org/freedesktop/gstreamer/GarbageCollectionTest.java
@@ -23,6 +23,7 @@
 import static org.junit.Assert.assertTrue;
 
 import java.lang.ref.WeakReference;
+import java.util.List;
 
 import org.junit.After;
 import org.junit.AfterClass;
@@ -76,16 +77,45 @@ public void testBin() throws Exception {
         
         assertEquals("source not returned", e1, bin.getElementByName("source"));
         assertEquals("sink not returned", e2, bin.getElementByName("sink"));
-        WeakReference<Element> binRef = new WeakReference<Element>(bin);
+        GCTracker binTracker = new GCTracker(bin);
         bin = null;
-        assertTrue("Bin not garbage collected", GCTracker.waitGC(binRef));
-        WeakReference<Element> e1Ref = new WeakReference<Element>(e1);
-        WeakReference<Element> e2Ref = new WeakReference<Element>(e2);
+        assertTrue("Bin not garbage collected", binTracker.waitGC());
+        assertTrue("Bin not destroyed", binTracker.waitDestroyed());
+        GCTracker e1Tracker = new GCTracker(e1);
+        GCTracker e2Tracker = new GCTracker(e2);
         e1 = null;
         e2 = null;
         
-        assertTrue("First Element not garbage collected", GCTracker.waitGC(e1Ref));
-        assertTrue("Second Element not garbage collected", GCTracker.waitGC(e2Ref));
+        assertTrue("First Element not garbage collected", e1Tracker.waitGC());
+        assertTrue("First Element not destroyed", e1Tracker.waitDestroyed());
+        assertTrue("Second Element not garbage collected", e2Tracker.waitGC());
+        assertTrue("Second Element not destroyed", e2Tracker.waitDestroyed());
+        
+    }
+    
+    @Test
+    public void testBinParsed() throws Exception {
+        Bin bin = Gst.parseBinFromDescription("fakesrc name=source ! fakesink name=sink", false);
+        int binRefCount = bin.getRefCount();
+        List<Element> children = bin.getElements();
+        assertEquals("Iteration increased Bin refcount", binRefCount, bin.getRefCount());
+        assertEquals("Wrong number of child elements", 2, children.size());
+        Element e1 = children.get(0);
+        Element e2 = children.get(1);
+        GCTracker binTracker = new GCTracker(bin);
+        bin = null;
+        assertTrue("Bin not garbage collected", binTracker.waitGC());
+        assertTrue("Bin not destroyed", binTracker.waitDestroyed());
+        GCTracker e1Tracker = new GCTracker(e1);
+        GCTracker e2Tracker = new GCTracker(e2);
+        children = null;
+        e1 = null;
+        e2 = null;
+        
+        assertTrue("First Element not garbage collected", e1Tracker.waitGC());
+        assertTrue("First Element not destroyed", e1Tracker.waitDestroyed());
+        assertTrue("Second Element not garbage collected", e2Tracker.waitGC());
+        assertTrue("Second Element not destroyed", e2Tracker.waitDestroyed());
         
     }
     @Test
