[vlc-devel] libvlc issue: how to determine which char* results should be freed?

[Olivier Aubert]

Hello

This is a question to VLC devs, not directly python-related.

I am integrating the patch proposed in the report below. However, I have
an issue when trying to determine an appropriate rule for freeing char*
results. The following libvlc functions return a char*. From a quick
overview, the result of the first 5 functions should not be freed, since
they are statically allocated, while the rest should:

libvlc_errmsg
libvlc_get_version
libvlc_get_compiler
libvlc_get_changeset
libvlc_event_type_name

libvlc_media_discoverer_localized_name
libvlc_media_get_mrl
libvlc_media_get_meta
libvlc_video_get_aspect_ratio
libvlc_video_get_crop_geometry
libvlc_video_get_marquee_string
libvlc_audio_output_device_longname
libvlc_audio_output_device_id
libvlc_vlm_show_media

Short of reading the source code, there is no way to determine which
return values should be freed and which should not. Only the
libvlc_video_get_aspect_ratio specifies in its comment that "the result
must be released with free() or libvlc_free()".

To address the freeing issue, I can either hardcode the function names
in the python generator (as proposed in the patch), or find a way to
appropriately decorate/comment the functions so that the include parser
can get the information.

The second approach looks cleaner to me, since new functions, if
appropriately documents, would be properly wrapped without having to
also modify the python generator. Moreover, it also documents the
behaviour for other libvlc users.

Are there any opinions on this? And any advice on how to best
decorate/comment this behaviour?

Regards,
Olivier



On Fri, 2011-04-15 at 17:00 +0000, VLC wrote:
> #4678: Memory leak in python binding's libvlc_media_get_mrl
> -----------------------------+----------------------------------------------
>  Reporter:  geoffs           |        Owner:  oaubert      
>      Type:  defect           |       Status:  new          
>  Priority:  normal           |    Milestone:  Bugs paradize
> Component:  Python bindings  |      Version:  master       
>  Severity:  normal           |     Keywords:               
>  Platform:  all              |   Difficulty:  unknown      
>       Wip:  Not started      |  
> -----------------------------+----------------------------------------------
>  I think the python bindings leak memory when the underlying native
>  function returns a char* that the caller is meant to free. The specific
>  function I'm looking at is {{{libvlc_media_get_mrl}}} which returns a
>  char* that is allocated by strdup in {{{input_item_GetURI}}}. There are
>  probably others though.
> 
>  I've attached a python script that will call get_mrl over and over again.
>  While running this on either Ubuntu 10.8 amd64 or Debian Squeeze i386, I
>  can watch the memory usage of the process climb steadily. I'm running VLC
>  1.1.4 on the Ubuntu machine and 1.1.3 on Debian, and I've tried using
>  different versions of vlc.py including the latest generated one in git.
> 
>  I've looked in vlc.py and I think the problem is that ctypes is told that
>  the native function returns {{{ctypes.c_char_p}}}, so ctypes will create a
>  python string object copy of the string, but it has no idea that it should
>  free the char*.
> 
>  I've tried to hack my copy of vlc.py to import libc, change the return
>  type of the function to ctypes.c_void_p, use ctypes.string_at() to create
>  the python string before calling libc.free on the pointer. This appears to
>  fix the leak in the simple script I attached but I'm getting segfaults in
>  free in a more complicated app. Those might be related to other problems
>  though, because I get different segfaults without the fix in other parts
>  of the code.
> 
>  I will continue working on my program to see if I can find the cause of
>  these segfaults. Is there a better way to free these returned char* values
>  with ctypes? I noticed libvlc_free, which could be more portable, but it's
>  not in my version of VLC.
> 
>  Plus, eventually the binding generator needs to be modified and needs a
>  way to identify which functions need special treatment.
>