From cc761eae6b4bec5b535cc24bbdcae44ad503594b Mon Sep 17 00:00:00 2001 From: Neil C Smith Date: Fri, 17 Apr 2020 16:04:36 +0100 Subject: [PATCH 1/2] Update PromiseTest and add test for issue 176. --- .../freedesktop/gstreamer/PromiseTest.java | 62 ++++++++++++------- 1 file changed, 39 insertions(+), 23 deletions(-) diff --git a/test/org/freedesktop/gstreamer/PromiseTest.java b/test/org/freedesktop/gstreamer/PromiseTest.java index 33c34b1d..26dd292f 100644 --- a/test/org/freedesktop/gstreamer/PromiseTest.java +++ b/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,6 +20,8 @@ */ 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; @@ -25,9 +29,9 @@ 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,9 +116,8 @@ 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(); @@ -127,15 +125,33 @@ public void testDispose() { @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(); } } From cce4cf58a5241f46f72e40e8fd42cf751a84f31b Mon Sep 17 00:00:00 2001 From: Neil C Smith Date: Fri, 17 Apr 2020 16:08:41 +0100 Subject: [PATCH 2/2] Fix issue 176 - Promise change listener being garbage collected. Keep GstCallback in field reference (which in turn holds reference to user supplied PROMISE_CHANGE listener). Both must live as long as Promise itself. --- src/org/freedesktop/gstreamer/Promise.java | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/org/freedesktop/gstreamer/Promise.java b/src/org/freedesktop/gstreamer/Promise.java index 34d5fa03..dd5e798f 100644 --- a/src/org/freedesktop/gstreamer/Promise.java +++ b/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; } /**