[compiz] Dbus plugin problem: Setting reset

[David Reveman]

On Tue, 2007-05-15 at 15:45 +0200, Danny Baumann wrote:
> Hi,
> 
> > > since quite some time I encounter a problem which I believe is in the
> > > dbus plugin: Sometimes when I load or unload plugins by changing the
> > > configuration, all settings of all plugins are reset. At the same time,
> > > the following message appears multiple times in the terminal I started
> > > Compiz from (which makes me believe the problem is in the dbus plugin):
> > >
> > > A handler is already registered for the path starting with path[0] =
> > > "org"
> > >   
> > 
> > These warnings were introduced with the dbus introspection, I
> > doubt they are related to your problem.
> > 
> > They are just warnings because some paths are being reregistered.
> 
> First let me say: That's correct - it really was coincidence. ;-)
> 
> > > I haven't been able to track it down yet because it happens rarely (but
> > > regularly).
> > > After this happens, the only ways to restore my custom config are to
> > > reload Compiz or to 'touch' each single setting ('touch' meaning change
> > > it and change it back).
> > > This has happened to me using both ini and ccp as setting storage
> > > backends, so I doubt it's related to that.
> 
> I finally have found the reason for the problem: It is wrapping into
> initPluginFor(Display|Screen) while executing initPluginFor(Display|
> Screen).
> Let me show this with the call stack. 'storage' in this example is the
> option storage plugin.
> 
> - d->initPluginForDisplay (plugin.c:300) is called at some point for the
> storage plugin
> - this calls core's initPluginForDisplay, which calls initDisplay
> (plugin.c:189)
> - storage WRAPs into initPluginForDisplay
> - initPluginForDisplay is called for the next plugin (e.g. dbus ;-) )
> - storageInitPluginForDisplay is called
> - storage UNWRAPs
> - core's initPluginForDisplay is called
> - initDisplay for the plugin is called
> - plugin WRAPs into initPluginForDisplay
> - storage WRAPs into initPluginForDisplay again
> 
> This is the reason why the pointer to storageInitPluginForDisplay will
> get lost afterwards and newly loaded plugins thus won't get their
> settings initialized. 
> 
> So in short: We are WRAPping a function while executing it, which is
> bad(TM). Does anyone have a suggestion how to fix it?

Yes, that's bad. The appropriate fix seems to be to move the plugin
initialization out of initPluginForDisplay and initPluginForScreen. I've
attached a patch that does this, let me know how it works.

Thanks for catching this one,

- David
-------------- next part --------------
A non-text attachment was scrubbed...
Name: plugin-init-fix.diff
Type: text/x-patch
Size: 2375 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/compiz/attachments/20070515/0d637b4d/attachment-0001.bin