Skip to content

Conversation

@kezhuw
Copy link
Contributor

@kezhuw kezhuw commented Nov 2, 2019

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.

@neilcsmith-net
Copy link
Member

Thanks! But not sure this is the right fix. Have you tried changing the underlying mapping in GstPromiseAPI to

void gst_promise_reply(Promise promise, @Invalidate Structure s);

@kezhuw
Copy link
Contributor Author

kezhuw commented Nov 5, 2019

@neilcsmith-net You are right, after dove into https://gitlab.freedesktop.org/gstreamer/gstreamer/blob/master/gst/gstpromise.c#L162, I think @Invalidate is the right fix, since there are chances the reply got swallow due to interrupted.

@kezhuw kezhuw force-pushed the disown-promise-reply branch from 235dbd5 to 872f19e Compare November 5, 2019 14:59
@kezhuw kezhuw changed the title Disown structure after ownership transferred to promise in Promise.reply Invalidate structure after ownership transferred to promise in Promise.reply Nov 5, 2019
@kezhuw kezhuw force-pushed the disown-promise-reply branch from 872f19e to bb47f25 Compare November 5, 2019 16:43
…e.reply

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.
@kezhuw kezhuw force-pushed the disown-promise-reply branch from bb47f25 to ffef4a8 Compare November 19, 2019 00:47
@neilcsmith-net neilcsmith-net added this to the 1.2 milestone Apr 8, 2020
@neilcsmith-net neilcsmith-net merged commit 83f7910 into gstreamer-java:master Apr 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants