[Rhythmbox-devel] RBShellPlayer and MonkeyMediaPlayer finalization bug

[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?