After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 788465 - baseparse: memory leak in error code path
baseparse: memory leak in error code path
Status: RESOLVED INCOMPLETE
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
1.13.x
Other Linux
: Normal normal
: NONE
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-10-03 11:45 UTC by Renu
Modified: 2018-01-19 18:18 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Please find the attached Patch. (905 bytes, patch)
2017-10-05 05:49 UTC, Renu
reviewed Details | Review

Description Renu 2017-10-03 11:45:36 UTC
Hi,

I found memory leak, in gstbaseparse.c file when there is no caps created.

line 2402:

  /* should have caps by now */
  if (!gst_pad_has_current_caps (parse->srcpad))
    goto no_caps;


Solution:-


  /* should have caps by now */
  if (!gst_pad_has_current_caps (parse->srcpad))
{
    gst_buffer_unref (buffer);
    goto no_caps;
}

Please review .


Regards,
Renu
Comment 1 Tim-Philipp Müller 2017-10-03 12:02:12 UTC
Thanks for the bug report.

Please could you make a patch in git format-patch format?

https://gstreamer.freedesktop.org/documentation/contribute/index.html#how-to-submit-patches
Comment 2 Renu 2017-10-05 05:49:21 UTC
Created attachment 360946 [details] [review]
Please find the attached Patch.

Please find the attached Patch.

Please review and verify.
Comment 3 Tim-Philipp Müller 2017-10-05 08:41:39 UTC
Comment on attachment 360946 [details] [review]
Please find the attached Patch.

Not clear to me that this is correct.

The docs say that this function takes ownership of the frame's buffer, but not the frame. However, we only take ownership of the frame's buffer later in the code as far as I can see.

If we unref the buffer here, we need to clear it from the frame I think, otherwise we leave a dangling pointer around.

But to me it seems like it should be fine to leave the buffer in the frame in the error case, and then it should be unreffed when the frame gets freed.

It's not clear to me that this needs fixing at all.
Comment 4 Tim-Philipp Müller 2018-01-19 18:18:31 UTC
Closing. Please feel free to re-open if you can explain in more details or have a test case that demonstrates a leak. Thanks!