Merge lp://qastaging/~alan-griffiths/compiz-core/rework-updatePlugins into lp://qastaging/compiz-core
| Status: | Merged |
|---|---|
| Merged at revision: | 3075 |
| 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 | Approve | ||
|
Review via email:
|
|||
This proposal supersedes a proposal from 2012-03-28.
Description of the change
Avoid graphics corruption and hangs on compiz start-up by ensuring that
plugins don't get initialized, de-initialized and re-initialzed during
start-up. (LP: #963093) (LP: #963633)
The multiple init/fini/init calls occured when compiz was asked to load an
invalid plugin name. This occurred most recently as plugins "bailer" and
"detection" were left in some peoples' configs while the plugins themselves
no longer exist in the current compiz release.
The source of the graphics corruption and hangs has been found to be a
problem in the composite and/or opengl plugins. One or both of them are unsafe
to init/fini multiple times without a full compiz restart. So the root cause
is not exactly known yet. However composite and opengl are not alone; many
plugins have bugs with init/fini/init sequences, so it is valuable to fix
the start-up plugin ordering as this does.
Essentially this fix works by remembering which plugins don't exist at all
and putting them on a black list. Then subsequent updates check the blacklist
and know they should never include those failed plugins in testing whether
the active plugin lists have changed.
The final part of the fix is to remove a rendundant call to updatePlugins from
EventManager::init. It is not required as main tells PluginManager when to
load plugins on startup.
IMPORTANT NOTE FOR UBUNTU PACKAGING
In downstream ubuntu branches the DEFAULT_PLUGINS list in "debian/rules" also causes multiple plugin loads on start-up and prevents this fix from working! The solution is to change DEFAULT_PLUGINS to just "ccp".
ORIGINAL DESCRIPTION:
Some test cases to cover what we need.
Combined Daniel's "blacklist" with some cleanup of the code and ignoring any non-existent plugins entirely.
| Daniel van Vugt (vanvugt) wrote | # |
| Daniel van Vugt (vanvugt) wrote | # |
| Daniel van Vugt (vanvugt) wrote | # |
| Daniel van Vugt (vanvugt) wrote | # |
| 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.