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 703980 - sink: fix memory leak on unchecked uploader ptr
sink: fix memory leak on unchecked uploader ptr
Status: RESOLVED FIXED
Product: gstreamer-vaapi
Classification: Other
Component: general
git master
Other Windows
: Normal normal
: ---
Assigned To: gstreamer-vaapi maintainer(s)
gstreamer-vaapi maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2013-07-11 06:42 UTC by congx.zhong
Modified: 2013-08-26 11:06 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
0001-vaapisink-fix-memory-leak-on-unchecked-uploader-ptr (971 bytes, patch)
2013-07-11 06:42 UTC, congx.zhong 		none Details | Review

Description congx.zhong 2013-07-11 06:42:04 UTC 
Created attachment 248905 [details] [review]
0001-vaapisink-fix-memory-leak-on-unchecked-uploader-ptr

Use gst_vaapisink_ensure_uploader instead of unsafe call
 on gst_vaapi_uploader_new
Comment 1 Gwenole Beauchesne 2013-07-11 09:15:39 UTC 
Review of attachment 248905 [details] [review]:

The patch looks good, but the real question would be why are we calling _start() twice without a _stop() in between? Adding this patch right now would hide the real issue. Please try to investigate more this one: why is the patch needed?
Comment 2 Wind Yuan 2013-07-11 09:41:42 UTC 
Hi,
  this patch was fixed on 0.4-branch, I'm not sure master branch have the same issue.
  the reason is 
    step 1. gst_vaapisink_get_caps was called first, which called gst_vaapisink_ensure_uploader to new a uploader.
    step 2. After step 1, gst_vaapisink_start was called, it doesn't check *sink->uploader* pointer but directly new a uploader and caused the memory leak.
Comment 3 Gwenole Beauchesne 2013-07-11 10:56:34 UTC 
(In reply to comment #2)

>   this patch was fixed on 0.4-branch, I'm not sure master branch have the same
> issue.
>   the reason is 
>     step 1. gst_vaapisink_get_caps was called first, which called
> gst_vaapisink_ensure_uploader to new a uploader.
>     step 2. After step 1, gst_vaapisink_start was called, it doesn't check
> *sink->uploader* pointer but directly new a uploader and caused the memory
> leak.

Thanks, that's clearer.
Comment 4 Gwenole Beauchesne 2013-08-26 11:06:18 UTC 
Fixed in git master branch. Thanks.
Comment 5 Gwenole Beauchesne 2013-08-26 11:06:27 UTC 
commit e5a50af2ae9c8d82c749854460f87f18e3a64640
Author: Wind Yuan <feng.yuan@intel.com>
Date:   Tue May 14 15:19:04 2013 +0800

    vaapisink: fix memory leak of GstVaapiUploader instance.
    
    Make sure gst_vaapisink_ensure_uploader() checks for the existence
    of a former GstVaapiUploader instance prior to forcibly creating a
    new one.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=703980