Red Hat Bugzilla – Bug 991149
Refactor Plugin Container Services initialization
Last modified: 2014-05-16 05:16:59 EDT
Created attachment 781694 [details]
Patch for master (69656f42348a)
See Bug 949065
From the patch notes:
Since there have been other race conditions in the past (as documented in the
code in a few spots), it feels there is an overall design issue at fault:
Objects are initialized in arbitrary order, enforced by the use of 'listeners',
but this was done as a reaction to what bugs were found.
The solution employed here is to use constructors and immutability when
possible. Handles to other components should be done using object fields, not
an object registry/singleton pattern. I tried to eliminate the use of
PluginContainer.getInstance().getXXX(), since in many cases references could be
null depending on the order of how objects are initialized. It also makes clear
what objects really depend on what objects. The use of 'PC.getInstance()'
should probably be entirely removed, since it's bug-prone, but I'll leave the
rest of the cleanup for later.
Testability is generally improved, as test/mock instances can simply be
'injected' using the constructor, not by having to create methods
that get overridden.
Created attachment 786683 [details]
patch rebased on master (69656f4234)
Changed the title to make it clear it's not about agent plugin component API - in case it was unclear for anybody other than me :)
Finished to apply the patch on master (many conflicts and changes in some test classes outside the plugin container module needed).
The PC module test suite passes
Currently smoke testing. Looks good so far.
* probe job passing
* api-check job passing
* in depth review
(In reply to Thomas Segismont from comment #3)
> Finished to apply the patch on master
It's currently in a bug branch (bug/991149)
Sorry about the patch complication. I tried my best to keep the changes in sync, at least at the moment when I submitted my patch. It may have worked better to apply the changes to the commit I patched on top of, then rebased the work.
Actually looks like there's a tool for that: https://github.com/niklasf/git-rebase-patch
Still I realize my changes were fairly invasive so it might not have saved any significant work. :-(
Feel free to send a patch back that needs attention.
Merged in master
Author: Elias Ross <firstname.lastname@example.org>
Date: Thu Nov 21 15:45:20 2013 +0100
Bulk closing of 4.10 issues.
If an issue is not solved for you, please open a new BZ (or clone the existing one) with a version designator of 4.10.
Reopening this - this introduced some very bad regressions. See bug #1096332
Seems to have been a false alarm. Closing again, the regressions weren't confirmed.