Merge pull request #200 from neilcsmith-net/issue-176

Issue 176 - Promise change listener being garbage collected

Author: neilcsmith-net

src/org/freedesktop/gstreamer/Promise.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019 Neil C Smith
+ * Copyright (c) 2020 Neil C Smith
  * Copyright (c) 2018 Vinicius Tona
  * Copyright (c) 2018 Antonio Morales
  * 
@@ -38,6 +38,8 @@ public class Promise extends MiniObject {
 
     public static final String GTYPE_NAME = "GstPromise";
 
+    private GstCallback changeFunction;
+    
     /**
      * Creates a new instance of Promise. This constructor is used internally.
      *
@@ -62,12 +64,17 @@ public Promise() {
      * {@link Promise} is changed
      */
     public Promise(final PROMISE_CHANGE listener) {
-        this(Natives.initializer(GSTPROMISE_API.ptr_gst_promise_new_with_change_func(new GstCallback() {
-            @SuppressWarnings("unused")
+        this(new GstCallback() {
             public void callback(Promise promise, Pointer userData) {
                 listener.onChange(promise);
             }
-        }, null, null)));
+        });
+    }
+    
+    private Promise(GstCallback callback) {
+        this(Natives.initializer(GSTPROMISE_API
+                .ptr_gst_promise_new_with_change_func(callback, null, null)));
+        this.changeFunction = callback;
     }
 
     /**

test/org/freedesktop/gstreamer/PromiseTest.java

@@ -1,4 +1,6 @@
 /* 
+ * Copyright (c) 2020 Neil C Smith
+ * Copyright (c) 2019 Kezhu Wang
  * Copyright (c) 2018 Antonio Morales
  * 
  * This file is part of gstreamer-java.
@@ -18,16 +20,18 @@
  */
 package org.freedesktop.gstreamer;
 
+import java.util.concurrent.atomic.AtomicBoolean;
+
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 import org.freedesktop.gstreamer.glib.Natives;
 import org.freedesktop.gstreamer.lowlevel.GPointer;
 import org.freedesktop.gstreamer.lowlevel.GType;
+import org.freedesktop.gstreamer.util.TestAssumptions;
 import org.junit.BeforeClass;
 import org.junit.AfterClass;
-import org.junit.Before;
 import org.junit.Test;
 
 public class PromiseTest {
@@ -37,7 +41,7 @@ public PromiseTest() {
 
     @BeforeClass
     public static void setUpClass() throws Exception {
-        Gst.init(Gst.getVersion(), "PromiseTest", new String[] {});
+        Gst.init(Gst.getVersion(), "PromiseTest");
     }
 
     @AfterClass
@@ -47,9 +51,8 @@ public static void tearDownClass() throws Exception {
 
     @Test
     public void testReply() {
-        if (!Gst.testVersion(1, 14)) {
-            return;
-        }
+        TestAssumptions.requireGstVersion(1, 14);
+        
         Promise promise = new Promise();
 
         promise.reply(null);
@@ -61,9 +64,8 @@ public void testReply() {
 
     @Test
     public void testInterrupt() {
-        if (!Gst.testVersion(1, 14)) {
-            return;
-        }
+        TestAssumptions.requireGstVersion(1, 14);
+        
         Promise promise = new Promise();
         promise.interrupt();
 
@@ -74,9 +76,8 @@ public void testInterrupt() {
 
     @Test
     public void testExpire() {
-        if (!Gst.testVersion(1, 14)) {
-            return;
-        }
+        TestAssumptions.requireGstVersion(1, 14);
+        
         Promise promise = new Promise();
         promise.expire();
 
@@ -87,9 +88,8 @@ public void testExpire() {
 
     @Test
     public void testInvalidateReply() {
-        if (!Gst.testVersion(1, 14)) {
-            return;
-        }
+        TestAssumptions.requireGstVersion(1, 14);
+        
         Promise promise = new Promise();
         Structure data = new Structure("data");
 
@@ -101,9 +101,8 @@ public void testInvalidateReply() {
 
     @Test
     public void testReplyData() {
-        if (!Gst.testVersion(1, 14)) {
-            return;
-        }
+        TestAssumptions.requireGstVersion(1, 14);
+        
         Promise promise = new Promise();
         Structure data = new Structure("data", "test", GType.UINT, 1);
         GPointer pointer = Natives.getPointer(data);
@@ -117,25 +116,42 @@ public void testReplyData() {
 
     @Test
     public void testDispose() {
-        if (!Gst.testVersion(1, 14)) {
-            return;
-        }
+        TestAssumptions.requireGstVersion(1, 14);
+        
         Promise promise = new Promise();
         promise.interrupt();
         promise.dispose();
     }
 
     @Test
     public void testDisposeWithChangeFunc() {
-        if (!Gst.testVersion(1, 14)) {
-            return;
-        }
+        TestAssumptions.requireGstVersion(1, 14);
+        
+        Promise promise = new Promise(new Promise.PROMISE_CHANGE() {
+            @Override
+            public void onChange(Promise promise) {
+            }
+        });
+        promise.interrupt();
+        promise.dispose();
+    }
+    
+    @Test
+    public void testChangeFunctionGC() {
+        TestAssumptions.requireGstVersion(1, 14);
+        
+        final AtomicBoolean onChangeFired = new AtomicBoolean(false);
+        
         Promise promise = new Promise(new Promise.PROMISE_CHANGE() {
             @Override
             public void onChange(Promise promise) {
+                onChangeFired.set(true);
             }
         });
+        System.gc();
+        System.gc();
         promise.interrupt();
+        assertTrue("Promise Change callback GC'd", onChangeFired.get());
         promise.dispose();
     }
 }