Created attachment 21620 [details] [review]
Probable fix

Mark: There seems to be a timing issue for the occurance of this
bug. Noticed that the gconf is returning FALSE for the 
display_all_workspaces key when the "applet_id" is re-used in 
"/app/panel/profiles/default/applets/applet_id/prefs".
Doing a gconf_client_remove_dir() appears solves the problem. It seems
to have fixed tasklist max and min size sometimes going 0 as well.

I'm really glad you tracked this down. I've committed a similar patch
to HEAD:

2003-12-02  Mark McLoughlin  <mark@skynet.ie>
                                                                     
                                                                         
        Make sure to stop monitoring the applet's GConf directory so
        that if we remove an applet the directory goes away completely
        before. Based a patch from Vijaykumar Patwari in #127285.
                                                                     
                                                                         
        * panel-applet.c:
        (panel_applet_add_preferences): don't start monitoring the
        dir here.
        (panel_applet_set_preferences_key): do it here and also remove
        the dir when we're unsetting.
        (panel_applet_finalize): unset the prefs key so the dir isn't
        monitored anymore.
        (panel_applet_item_handler_get_object): use set_prefs_key().
        (panel_applet_instance_init): initialize the GConfClient here.

Mark: I can still reproduce the problem. So re-opening the bug

Can still reproduce with HEAD or with an older build?

Vijay: ping?

Sorry for the late response, apologies!. Yes i can still reproduce
with HEAD.

I just reproduced this bug a few times on Fedora Core 1 with
gnome-panel-2.4.0-3.

*** Bug 140277 has been marked as a duplicate of this bug. ***

Created attachment 29303 [details] [review]
Patch Fixes the bug.

ping : can this patch be committed ?

It looks ok, but I'd prefer if Mark could look at it because I'm not a
libpanel-applet expert.

Mark : ping ?

Vijay: I don't understand how/why this fixes the bug. Could you explain?

All it seems to do is delay the add_dir() until we do add_preferences().

Vijay: ping ?

Oops sorry Mark, Vincent, I missed out this mail.

Basically it looks like, it is watching the directory without having gconf
values set for the keys.

So, this fix first associates gconf values for the schema key, and then add
directory to the list of directories that GConfClient will watch.

Mark: is it clearer for you now?

Could bug #163093 be related to this?

Vincent: I have updated the bug #163093 with comments. Looks to be similar, but
not very sure :).

I can reproduce with HEAD.

*** Bug 163093 has been marked as a duplicate of this bug. ***

Note for myself: what might be going on here is that when we do gconf_client_remove_dir(), the cache of the gconf client might not remove the data for the directory. Or something like this. Might be a gconf bug.

I made a patch for gconf and it fixes it. Will attach it in a few minutes.

Created attachment 56635 [details] [review]
Remove entries from cache when removing a directory

So, I think what's happening here is that the cache contained some old entries that didn't get updated. Some background:

 + when you remove an applet from the panel, the gconf directory containing the prefs of the applet is removed
 + wnck-applet continues running if you remove the workspace switcher since it also contains some other applets
 + when an applet is removed, we tell the gconf library we're not interested in the gconf directory of this applet anymore, but it keeps in cache the previous values
 + when we reuse the same applet number in the panel, and old gconf directory is used for the pref. It doesn't contain anything (yet), but in this case, values are still in the cache

Solution: when we tell the gconf client we're not interested in a directory anymore, remove values from the cache. It works for me.

Nice catch vuntz - my main suggestion would be to use gconf_key_is_below() in clear_dir_cache_foreach() ... i.e. /foofoo isn't under /foo

Another problem is that we are be removing too much from the cache - i.e. if we do

  gconf_client_add_dir (client, "/foo/bar");

  entry = get (client, "/foo/bar/1");

  gconf_client_add_dir (client, "/foo");
  gconf_client_remove_dir (client, "/foo");

then the entry for "/foo/bar/1" isn't cache before, even though it clearly should be. The only problem that would cause, though, is that the value would have to be re-fetched.

That's a minor issue compared to the fact that we seem to be removing the notify for "/foo/bar" when we remove "/foo" - see foreach_setup_overlap() 

So ... the patch looks good to me - make it use gconf_key_is_below() and commit

s/isn't cache before/isn't cached anymore/

Thanks, committed in HEAD. Should I commit to gnome-2-12?

Sure, sounds sensible for gnome-2-12