Merge lp://qastaging/~alan-griffiths/compiz-core/rework-updatePlugins into lp://qastaging/compiz-core
| Status: | Superseded |
|---|---|
| Proposed branch: | lp://qastaging/~alan-griffiths/compiz-core/rework-updatePlugins |
| Merge into: | lp://qastaging/compiz-core |
| Diff against target: |
686 lines (+439/-107) (has conflicts) 4 files modified
include/core/plugin.h (+1/-1) src/privatescreen.h (+6/-0) src/privatescreen/tests/test-privatescreen.cpp (+352/-13) src/screen.cpp (+80/-93) Text conflict in src/privatescreen/tests/test-privatescreen.cpp |
| To merge this branch: | bzr merge lp://qastaging/~alan-griffiths/compiz-core/rework-updatePlugins |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Daniel van Vugt | Disapprove | ||
|
Review via email:
|
|||
This proposal has been superseded by a proposal from 2012-03-29.
Description of the change
Try to avoid finalize/re-init based on...
/1/ I think in the general case we need to be able to finalize and
re-initialize plugins. Plugins that can't cope are broken, I don't
think there's a way around that.
Scenario: plugin B depends on plugin A - whenever plugin A is unloaded
and reloaded plugin B must be finalized and reinitialized. (Example A:
composite, B:opengl - opengl has pointers to composite which would go
stale when composite is finalized.)
/2/ I don't grok the opengl plugin well enough to fix it quickly.
/3/ updatePlugins does more finalize/
necessary - because the logic is pants.
/3.1/ One test case we have is:
plugin: A, B, C - where B fails to load, but A and C succeed. (NB:
there is a fine distinction here between a plugin failing to /load/ and
failing to /init/.)
Then, whenever updatePlugins runs for second or subsequent occasions,
it detects that currently loaded "AC" isn't desired "ABC" and finalizes
C before trying to reload B (which fails) before re-initializing C.
BUT there is no possible reason to do the finalization of C until it is
about to init B - which never happens because B fails to load.
FURTHER any plugin C that has successfully initialized can't (well, it
shouldn't!) have dependences on any plugin B that is not loaded or
initialized.
/3.2/ AFAICS we ONLY need to finalize plugins when we are unloading
plugins. And we need to do this from the end of the list to the plugin
in question. That is: if we have "ABC" loaded, then to unload B, we
first need to finalize C and then B. (C.f. /1/ above.)
/4/ Part of the problem therefore is that the CompPlugin class conflates
management of the plugin stack with initialization and finalization of
the plugins. (E.g. Just because some plugins are "popped" to insert a
new one, doesn't require them to be finalized. (C.f. 3.2 above))
| Daniel van Vugt (vanvugt) wrote | # |
Re: "Just because some plugins are "popped" to insert a new one, doesn't require them to be finalized."
This is untrue. Some plugins have loose dependencies on others and change their behaviour/features depending on what other plugins are loaded. Hence popped plugins must be fini'd and reinitialized.
Also, even if this branch works perfectly I would still be unhappy with the risk associated with such a large change in critical logic. Please don't hate me...
I'm looking at a much smaller/simpler solution. In fact I was designing it in my head as you designed this code. Will let you know if/when I think I have something that works.
In other news, the two critical bugs we're aiming to fix have had ZERO duplicates in the past two days. It sounds like the workarounds are working.