Re: [PATCH] [media-source] Add test to prevent segfault on browse_result_relay_cb

[Iago Toral Quiroga <itoral igalia com>]

Hi Joaquim,

thanks for the bug report. 

Checking out your patch I am a bit surprised, because that patch should
not be needed: browse_result_relay_cb should never be invoked with a
NULL source object. The source object is referenced (g_object_ref) when
a browse/search operation is started and unreferenced (g_object_unref)
when the operation is done (remaining==0 has been emitted by the
plugin). A plugin must not emit a new result after remaining=0 has been
sent, for that signals the end of the operation.

I can only think of these reasons for this to happen:

1) A plugin is emitting results after sending remaining == 0.
2) Someone is unreffing the source object somewhere twice.
3) There is some other kind of memory management problem somewhere

The g_return_if_fail in your patch might prevent the segfault, but it is
masquerading the real problem.

I would appreciate if you can provide backtrace information extracted
from gdb and any info you can get on the "remaining" parameter being
passed on to the browse_result_cb when this problem happens. Probably it
is better if you attach this information and your patch to bugzilla.

Also, since this is only happening when using rygel-grilo I wonder if
the source of the problem might live there. Another issue might be
related to you using Python, since AFAIK, the introspection support for
Grilo is not stable yet.

Iago

El vie, 14-05-2010 a las 10:08 +0200, Joaquim Rocha escribió:
> ---
>  src/grl-media-source.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/src/grl-media-source.c b/src/grl-media-source.c
> index 72ee3ef..aab9fdc 100644
> --- a/src/grl-media-source.c
> +++ b/src/grl-media-source.c
> @@ -625,6 +625,8 @@ browse_result_relay_cb (GrlMediaSource *source,
>  
>    g_debug ("browse_result_relay_cb");
>  
> +  g_return_if_fail (GRL_IS_MEDIA_SOURCE (source));
> +
>    brc = (struct BrowseRelayCb *) user_data;
>  
>    plugin_remaining = remaining;