Skip to content

Commit 83f7910

Browse files
authored
Invalidate structure after ownership transferred to promise in Promise.reply (#178)
If we don't invalidate after Promise.reply, we may sit in situations: 1. structure got freed while it is owned by promise, we can get hit from gst log: ``` (PromiseTest:13221): GStreamer-CRITICAL **: 10:28:01.594: gst_structure_free: assertion 'GST_STRUCTURE_REFCOUNT (structure) == NULL' failed ``` 2. structure got freed after its owning promise freed, we got double-free. 3. structure got used after freed due to interrupted before reply or owning promise freed.
1 parent 0e56e50 commit 83f7910

File tree

4 files changed

+45
-3
lines changed

4 files changed

+45
-3
lines changed

src/org/freedesktop/gstreamer/Promise.java

+4-1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
import static org.freedesktop.gstreamer.lowlevel.GstPromiseAPI.GSTPROMISE_API;
2121

22+
import org.freedesktop.gstreamer.glib.NativeObject;
2223
import org.freedesktop.gstreamer.lowlevel.GstAPI.GstCallback;
2324

2425
import com.sun.jna.Pointer;
@@ -86,7 +87,9 @@ public PromiseResult waitResult() {
8687
* {@link PromiseResult} state. If the promise has already been interrupted
8788
* than the replied will not be visible to any waiters
8889
*
89-
* @param structure the {@link Structure} to reply the promise with
90+
* @param structure the {@link Structure} to reply the promise with, caller
91+
* should not use this structure afterward as it is invalidated through
92+
* {@link NativeObject#invalidate()}
9093
*/
9194
public void reply(final Structure structure) {
9295
GSTPROMISE_API.gst_promise_reply(this, structure);

src/org/freedesktop/gstreamer/glib/Natives.java

+20
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,26 @@ public static GPointer getPointer(NativeObject obj) {
187187
return obj.getPointer();
188188
}
189189

190+
/**
191+
* Return whether underlying native pointer is owned by this object.
192+
*
193+
* @param obj native object which may hold a reference to native pointer
194+
* @return whether underlying native pointer is owned by this object
195+
*/
196+
public static boolean ownsReference(NativeObject obj) {
197+
return obj.handle.ownsReference();
198+
}
199+
200+
/**
201+
* Returns whether this object is valid or not.
202+
*
203+
* @param obj native object
204+
* @return whether this object is valid or not
205+
*/
206+
public static boolean validReference(NativeObject obj) {
207+
return obj.handle.getPointer() != null;
208+
}
209+
190210
/**
191211
* Increase the reference count of a {@link RefCountedObject}
192212
*

src/org/freedesktop/gstreamer/lowlevel/GstPromiseAPI.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.freedesktop.gstreamer.lowlevel.annotations.CallerOwnsReturn;
2929

3030
import com.sun.jna.Pointer;
31+
import org.freedesktop.gstreamer.lowlevel.annotations.Invalidate;
3132

3233
/**
3334
* GstPromise methods and structures
@@ -65,7 +66,7 @@ protected List<String> getFieldOrder() {
6566

6667
PromiseResult gst_promise_wait(Promise promise);
6768

68-
void gst_promise_reply(Promise promise, Structure s);
69+
void gst_promise_reply(Promise promise, @Invalidate Structure s);
6970
void gst_promise_interrupt(Promise promise);
7071
void gst_promise_expire(Promise promise);
7172

test/org/freedesktop/gstreamer/PromiseTest.java

+19-1
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,11 @@
1919
package org.freedesktop.gstreamer;
2020

2121
import static org.junit.Assert.assertEquals;
22+
import static org.junit.Assert.assertFalse;
2223
import static org.junit.Assert.assertTrue;
2324

25+
import org.freedesktop.gstreamer.glib.Natives;
26+
import org.freedesktop.gstreamer.lowlevel.GPointer;
2427
import org.freedesktop.gstreamer.lowlevel.GType;
2528
import org.junit.BeforeClass;
2629
import org.junit.AfterClass;
@@ -82,19 +85,34 @@ public void testExpire() {
8285
assertEquals("promise reply state not correct", promiseStatus, PromiseResult.EXPIRED);
8386
}
8487

88+
@Test
89+
public void testInvalidateReply() {
90+
if (!Gst.testVersion(1, 14)) {
91+
return;
92+
}
93+
Promise promise = new Promise();
94+
Structure data = new Structure("data");
95+
96+
assertTrue(Natives.ownsReference(data));
97+
promise.reply(data);
98+
assertFalse(Natives.ownsReference(data));
99+
assertFalse(Natives.validReference(data));
100+
}
101+
85102
@Test
86103
public void testReplyData() {
87104
if (!Gst.testVersion(1, 14)) {
88105
return;
89106
}
90107
Promise promise = new Promise();
91108
Structure data = new Structure("data", "test", GType.UINT, 1);
109+
GPointer pointer = Natives.getPointer(data);
92110

93111
promise.reply(data);
94112
assertEquals("promise state not in replied", promise.waitResult(), PromiseResult.REPLIED);
95113

96114
Structure result = promise.getReply();
97-
assertTrue("result of promise does not match reply", result.isEqual(data));
115+
assertEquals("result of promise does not match reply", pointer, Natives.getPointer(result));
98116
}
99117

100118
@Test

0 commit comments

Comments
 (0)