Re: [Rhythmbox-devel] RBShellPlayer and MonkeyMediaPlayer finalization bug

[Benjamin Otte <in7y118 public uni-hamburg de>]

Considering you can just use
g_signal_handlers_disconnect_matched (player->priv->mmplayer, 
G_SIGNAL_MATCH_DATA, 0, 0, NULL, NULL, player);
and be done with it - without getting that bug again when someone decides to 
ref the player at some other place - that sounds a lot better to me.

But I'm not the one to decide on that, I'm just the one doing it that way in 
all my code. :)

Benjamin


Quoting Ben Liblit <liblit@cs.berkeley.edu>:

> I am hot on the trail of a Rhythmbox crash-on-quit bug and could use 
> some advice from more seasoned Rhythmbox developers.  The bug appears to 
> manifest when the singleton RBShellPlayer instance and its contained 
> MonkeyMediaPlayer instance are finalized in an atypical order.
> 
> In the description that follows, I'm looking at the 0.6.5 source tree. 
> If any of this is no longer relevant in newer code, please let me know. 
>   I expect that most of it should still be around in some form or another.
> 
> In "shell/rb-shell.c", rb_shell_player_finalize() calls 
> g_object_unref(player->priv->mmplayer).  In most circumstances, this 
> drives the ref count on player->priv->mmplayer to zero and it is 
> reclaimed before that g_object_unref() call returns.
> 
> However, when the bug I'm hunting occurs, player->priv->mmplayer is 
> *not* reclaimed here.  Apparently there is at least one other 
> outstanding reference to that MonkeyMediaPlayer instance that keeps it 
> alive.  That instance will *eventually* be reclaimed, but not until 
> somewhat later.
> 
> In the meantime, though, that MonkeyMediaPlayer instance is still alive 
> and can still do things like invoke signal callbacks.  The RBShellPlayer 
> instance installed several callbacks during rb_shell_player_init(), and 
> all of those callbacks are still live, even though the RBShellPlayer 
> instance has already been freed.  I do see examples of the "info" signal 
> invoking info_available_cb(), where the RBShellPlayer callback argument 
> is a dangling pointer to that RBShellPlayer instance which has already 
> been freed.
> 
> I can see two ways to try fixing this.  I need advice on which one is 
> preferred.  One option is for rb_shell_player_finalize() to disconnect 
> all of the signal callbacks installed by rb_shell_player_init().  That 
> ensures that none of those callback functions will ever be given a 
> dangling pointer to an RBShellPlayer instance that has already been 
> finalized.
> 
> The other approach is to assert that the MonkeyMediaPlayer instance 
> should always be reclaimed within rb_shell_player_finalize().  The 
> question then becomes a matter of identifying which other piece of code 
> is holding a reference to player->priv->mmplayer and thereby preventing 
> it from being destroyed.
> 
> So what is the right strategy?  Disconnect the signal callbacks or 
> unravel the extra MonkeyMediaPlayer reference?
> _______________________________________________
> rhythmbox-devel mailing list
> rhythmbox-devel@gnome.org
> http://mail.gnome.org/mailman/listinfo/rhythmbox-devel
>