Re: [gupnp] [PATCH] Fix potential crash in resource_expire()
- From: Sven Neumann <s neumann raumfeld com>
- To: gupnp o-hand com
- Subject: Re: [gupnp] [PATCH] Fix potential crash in resource_expire()
- Date: Thu, 16 Dec 2010 14:44:33 +0100
Hi,
On Thu, 2010-12-16 at 14:56 +0200, Zeeshan Ali (Khattak) wrote:
> > On Mon, Nov 15, 2010 at 10:58 AM, Sven Neumann <s neumann raumfeld com> wrote:
> >> GSSDPResourceBrowser emits "resource-unavailable" before it has removed
> >> the resource from its cache. Now if the application changes the cache
> >> in response to this signal emission the code will crash.
> >>
> >> Fix this potential crash by emitting the signal after the resource
> >> has been removed from the cache.
>
> gupnp-universal-cp had started to randomly crash recently. The crash
> was happening just after a 'signal xxx invalid for instance xxx'
> warning from gobject. Running it as
> `G_DEBUG='fatal-warnings,fatal-criticals' gdb gupnp-universal-cp` I
> narrowed down the issue to the part which you changed with this patch
> recently:
>
> Program received signal SIGTRAP, Trace/breakpoint trap.
> g_logv (log_domain=<value optimized out>,
> log_level=G_LOG_LEVEL_WARNING, format=0xaf3500 "%s: signal id `%u' is
> invalid for instance `%p'",
> args1=0xbfffeaec "\226\063\257") at gmessages.c:563
> 563 g_private_set (g_log_depth, GUINT_TO_POINTER (depth));
> (gdb) bt
> #0 g_logv (log_domain=<value optimized out>,
> log_level=G_LOG_LEVEL_WARNING, format=0xaf3500 "%s: signal id `%u' is
> invalid for instance `%p'",
> args1=0xbfffeaec "\226\063\257") at gmessages.c:563
> #1 0x00b55b42 in g_log (log_domain=0xaefaa4 "GLib-GObject",
> log_level=G_LOG_LEVEL_WARNING,
> format=0xaf3500 "%s: signal id `%u' is invalid for instance `%p'")
> at gmessages.c:577
> #2 0x00add90d in g_signal_emit_valist (instance=0x828eed0,
> signal_id=208, detail=0, var_args=0xbfffec4c "`\006*\b\364\337\277")
> at gsignal.c:2931
> #3 0x00ade552 in g_signal_emit (instance=0x828eed0, signal_id=208,
> detail=0) at gsignal.c:3040
> #4 0x0083dd8b in resource_expire (user_data=0x824fd00) at
> gssdp-resource-browser.c:596
> #5 0x00b4b19c in g_timeout_dispatch (source=0x82bb1b8, callback=0x1,
> user_data=0x824fd00) at gmain.c:3688
> #6 0x00b4a9b5 in g_main_dispatch (context=0x8083e08) at gmain.c:2267
> #7 g_main_context_dispatch (context=0x8083e08) at gmain.c:2824
> #8 0x00b4ebc8 in g_main_context_iterate (context=0x8083e08,
> block=<value optimized out>, dispatch=1, self=0x8057048) at
> gmain.c:2902
> #9 0x00b4f107 in g_main_loop_run (loop=0xb4800c60) at gmain.c:3110
> #10 0x00277169 in IA__gtk_main () at gtkmain.c:1238
> #11 0x0804d29f in main (argc=1, argv=0xbfffef24) at main.c:119
>
> I reset the HEAD to the parent commit of this and so far I have
> failed to reproduce the issue. Are you sure your patch fixes a crash
> rather than the opposite? :)
Well, yes, it definitely fixed a crash for us. But I have now looked at
the code again and it looks like while it fixed a possible crash it also
introduced another. Sorry for that. I am attaching a patch that should
fix the problem. Could you please give it a test?
Sven
>From 7691277f3eed7b08dd332fa2853d88bfce054e14 Mon Sep 17 00:00:00 2001
From: Sven Neumann <s neumann raumfeld com>
Date: Thu, 16 Dec 2010 14:41:21 +0100
Subject: [PATCH] Fix issue introduced with commit 20dce239.
The Reource struct is not any longer valid at the point we emit the
signal, so better not access it there.
---
libgssdp/gssdp-resource-browser.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/libgssdp/gssdp-resource-browser.c b/libgssdp/gssdp-resource-browser.c
index 7d98c1e..10f7301 100644
--- a/libgssdp/gssdp-resource-browser.c
+++ b/libgssdp/gssdp-resource-browser.c
@@ -580,10 +580,12 @@ gssdp_resource_browser_get_active (GSSDPResourceBrowser *resource_browser)
static gboolean
resource_expire (gpointer user_data)
{
+ GSSDPResourceBrowser *resource_browser;
Resource *resource;
char *usn;
resource = user_data;
+ resource_browser = resource->resource_browser;
/* Steal the USN pointer from the resource as we need it for the signal
* emission.
@@ -593,7 +595,7 @@ resource_expire (gpointer user_data)
g_hash_table_remove (resource->resource_browser->priv->resources, usn);
- g_signal_emit (resource->resource_browser,
+ g_signal_emit (resource_browser,
signals[RESOURCE_UNAVAILABLE],
0,
usn);
--
1.7.1
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]