gstreamer-java / gst1-java-core

Java bindings for GStreamer 1.x
GNU Lesser General Public License v3.0
194 stars 72 forks source link

Fix leak of gst promise with custom change func #175

Closed kezhuw closed 4 years ago

kezhuw commented 4 years ago

Synchronize signatures of GstPromiseAPI.gst_promise_new_with_change_func and GstPromiseAPI.ptr_gst_promise_new_with_change_func to C's version, so that it would not get unknown user_data and notify, otherwise we will get coredump in gst_promise_free.

neilcsmith-net commented 4 years ago

Thanks! Will review ASAP.

neilcsmith-net commented 4 years ago

Not sure if the changes to the lowlevel mappings are required here?! You're also changing the version of Natives.initializer in use - potentially to the correct one.

kezhuw commented 4 years ago

The old invocation of Native.initializer(ptr, false, false) is where memory leak introduced. Following code creates a not owned promise in java.

Promise promise = new Promise(new Promise.PROMISE_CHANGE() {
    @Override
    public void onChange(Promise promise) {
    }
});

GstPromise * gst_promise_new_with_change_func (GstPromiseChangeFunc func, gpointer user_data, GDestroyNotify notify) has two additional parameters, while its java corresponding GstPromiseAPI.gst_promise_new_with_change_func doesn't. When we construct promise using java version, we pass only one argument, C version got random user_data, notify from registers or stacks. Thus in gst_promise_free we may got SIGBUS or SIGSEGV due to random notify.

Current version got no coredump due to memory leak, aka. no gst_promise_free at all.