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 770044 - rhythmbox-3.3.1 grilo plugin crashes on reload
rhythmbox-3.3.1 grilo plugin crashes on reload
Status: RESOLVED FIXED
Product: rhythmbox
Classification: Other
Component: Plugins (other)
3.4
Other Linux
: Normal major
: ---
Assigned To: RhythmBox Maintainers
RhythmBox Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-08-17 16:01 UTC by gnome.vrb
Modified: 2016-08-26 13:11 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
grilo: fix segfault due bad signal handler (3.42 KB, patch)
2016-08-25 13:30 UTC, Victor Toso
none Details | Review
-- (3.40 KB, patch)
2016-08-25 13:34 UTC, Victor Toso
accepted-commit_now Details | Review

Description gnome.vrb 2016-08-17 16:01:39 UTC
1. Open rhythmbox plugins window.

2. Toggle Enable / Disable grilo plugin

3. Rhythmbox crashes.
Comment 1 gnome.vrb 2016-08-17 16:02:10 UTC
Stack trace of crash:

gdb) bt
  • #0 __GI_raise
    at ../sysdeps/unix/sysv/linux/raise.c line 54
  • #1 __GI_abort
    at abort.c line 89
  • #2 g_assertion_message
  • #3 g_assertion_message_expr
    at ././glib/gtestutils.c line 2452
  • #4 g_hash_table_lookup
    at ././glib/ghash.c line 373
  • #5 g_hash_table_lookup
    at ././glib/ghash.c line 1147
  • #6 grilo_source_removed_cb
    at rb-grilo-plugin.c line 161
  • #10 <emit signal ??? on instance 0x19ec1f0 [GrlRegistry]>
    at ././gobject/gsignal.c line 3441
  • #11 grl_registry_unregister_source
  • #12 grilo_source_added_cb
    at rb-grilo-plugin.c line 153
  • #16 <emit signal ??? on instance 0x19ec1f0 [GrlRegistry]>
    at ././gobject/gsignal.c line 3441

Comment 2 Jonathan Matthew 2016-08-22 09:11:45 UTC
This doesn't look like it's grilo's fault.
Comment 3 Victor Toso 2016-08-25 13:30:12 UTC
Created attachment 334140 [details] [review]
grilo: fix segfault due bad signal handler

If restart the Grilo plugin, the signal handler for "source-added" and
"source-removed" will remain alive causing a crash as the previous
RBGriloPlugin does not exist anymore.

Backtrace:
 #0  0x00007f0f9af231c8 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:54
 #1  0x00007f0f9af2464a in __GI_abort () at abort.c:89
 #2  0x00007f0f9b7386a5 in g_assertion_message (domain=domain@entry=0x7f0f9b759fae "GLib", file=file@entry=0x7f0f9b75ec90 "/build/glib2.0-fJSoGg/glib2.0-2.48.1/./glib/ghash.c", line=line@entry=373,
     func=func@entry=0x7f0f9b75eed0 <__func__.10230> "g_hash_table_lookup_node", message=message@entry=0x3113c30 "assertion failed: (hash_table->ref_count > 0)")
     at ././glib/gtestutils.c:2429
 #3  0x00007f0f9b73873a in g_assertion_message_expr (domain=domain@entry=0x7f0f9b759fae "GLib", file=file@entry=0x7f0f9b75ec90 "/build/glib2.0-fJSoGg/glib2.0-2.48.1/./glib/ghash.c",
     line=line@entry=373, func=func@entry=0x7f0f9b75eed0 <__func__.10230> "g_hash_table_lookup_node", expr=expr@entry=0x7f0f9b75ebd8 "hash_table->ref_count > 0") at ././glib/gtestutils.c:2452
 #4  0x00007f0f9b701c4e in g_hash_table_lookup (hash_return=<synthetic pointer>, key=0x19ec730, hash_table=0x7f0f9ea9ca70) at ././glib/ghash.c:373
 #5  0x00007f0f9b701c4e in g_hash_table_lookup (hash_table=0x7f0f9ea9ca70, key=key@entry=0x19ec730) at ././glib/ghash.c:1147
 #6  0x00007f0f74024c93 in grilo_source_removed_cb (registry=<optimized out>, grilo_source=0x19ec730 [GrlBookmarksSource], plugin=0x277d370) at rb-grilo-plugin.c:161

Reported-by: vrishab <vrishab.in@gmail.com>
Comment 4 Victor Toso 2016-08-25 13:34:19 UTC
Created attachment 334143 [details] [review]
--

v1->v2
* indentation: tabs!
--

grilo: fix segfault due bad signal handler

If restart the Grilo plugin, the signal handler for "source-added" and
"source-removed" will remain alive causing a crash as the previous
RBGriloPlugin does not exist anymore.

Backtrace:
 #0  0x00007f0f9af231c8 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:54
 #1  0x00007f0f9af2464a in __GI_abort () at abort.c:89
 #2  0x00007f0f9b7386a5 in g_assertion_message (domain=domain@entry=0x7f0f9b759fae "GLib", file=file@entry=0x7f0f9b75ec90 "/build/glib2.0-fJSoGg/glib2.0-2.48.1/./glib/ghash.c", line=line@entry=373,
     func=func@entry=0x7f0f9b75eed0 <__func__.10230> "g_hash_table_lookup_node", message=message@entry=0x3113c30 "assertion failed: (hash_table->ref_count > 0)")
     at ././glib/gtestutils.c:2429
 #3  0x00007f0f9b73873a in g_assertion_message_expr (domain=domain@entry=0x7f0f9b759fae "GLib", file=file@entry=0x7f0f9b75ec90 "/build/glib2.0-fJSoGg/glib2.0-2.48.1/./glib/ghash.c",
     line=line@entry=373, func=func@entry=0x7f0f9b75eed0 <__func__.10230> "g_hash_table_lookup_node", expr=expr@entry=0x7f0f9b75ebd8 "hash_table->ref_count > 0") at ././glib/gtestutils.c:2452
 #4  0x00007f0f9b701c4e in g_hash_table_lookup (hash_return=<synthetic pointer>, key=0x19ec730, hash_table=0x7f0f9ea9ca70) at ././glib/ghash.c:373
 #5  0x00007f0f9b701c4e in g_hash_table_lookup (hash_table=0x7f0f9ea9ca70, key=key@entry=0x19ec730) at ././glib/ghash.c:1147
 #6  0x00007f0f74024c93 in grilo_source_removed_cb (registry=<optimized out>, grilo_source=0x19ec730 [GrlBookmarksSource], plugin=0x277d370) at rb-grilo-plugin.c:161

Reported-by: vrishab <vrishab.in@gmail.com>
Comment 5 gnome.vrb 2016-08-25 15:00:28 UTC
This fixes the crash. 

Tested on git rhythmbox. Thanks !
Comment 6 Hussam Al-Tayeb 2016-08-26 10:19:55 UTC
Why resolved as fixed? The patch was not commited.
Comment 7 Bastien Nocera 2016-08-26 11:58:53 UTC
Review of attachment 334143 [details] [review]:

> fix segfault due bad signal handler

Fix crash due to lingering signal handler

> If restart the Grilo plugin, the signal handler for "source-added" and
> "source-removed" will remain alive causing a crash as the previous
> RBGriloPlugin does not exist anymore.

If the grilo rhythmbox plugin is deactivated and reactivated, the signal...
... as the previous RBGriloPlugin instance does not exist anymore.

Looks good to me otherwise.

Would be nice to know why the sources aren't reaped though.
Comment 8 Victor Toso 2016-08-26 12:18:00 UTC
(In reply to Bastien Nocera from comment #7)
> Review of attachment 334143 [details] [review] [review]:
> 
> > fix segfault due bad signal handler
> 
> Fix crash due to lingering signal handler
> 
> > If restart the Grilo plugin, the signal handler for "source-added" and
> > "source-removed" will remain alive causing a crash as the previous
> > RBGriloPlugin does not exist anymore.
> 
> If the grilo rhythmbox plugin is deactivated and reactivated, the signal...
> ... as the previous RBGriloPlugin instance does not exist anymore.
> 
> Looks good to me otherwise.

Thanks, fixed.

> 
> Would be nice to know why the sources aren't reaped though.

Hm, they are removed after plugin deactivation (here). File a new bug?

I'll be pushing this one.
Comment 10 gnome.vrb 2016-08-26 13:11:36 UTC
(In reply to Hussam Al-Tayeb from comment #6)
> Why resolved as fixed? The patch was not commited.

Sorry about that.