Vojtech Szocs has posted comments on this change.
Change subject: userportal,webadmin: update branding manager
......................................................................
Patch Set 5: (1 inline comment)
....................................................
File
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/branding/BrandingManager.java
Line 80: /**
Line 81: * Get an instance of the {@code BrandingManager} with the default
ETC_DIR.
Line 82: * @return A {@code BrandingManager}
Line 83: */
Line 84: public static synchronized BrandingManager getInstance() {
OK, according to
http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html volatile
*can* be used to make the double-checked locking idiom working in JVM 5+ but
only *if* JVM implements volatile semantics properly. My bad, I missed the fact
that original article I posted didn't use volatile + synchronized but just
synchronized in most of their examples.
Also check out
http://avricot.com/blog/index.php?post/2010/05/11/Java-MultiThread-singleton-LocalThread-vs-Double-Check-Locked-(dcl)-vs-synchronized-vs-static
which presents all related strategies, including double-checked locking one.
Overall, I still think that for most cases, double-checked locking (using
volatile + synchronized) is unnecessarily complex and should be avoided. The
best way to implement singleton is via enum or via static field initialized
eagerly or via holder pattern. But in case we need to pass arguments to
singleton for its creation, using synchronized on method level is still
reasonable, in my opinion.
Line 85: if (instance == null) {
Line 86: instance = new BrandingManager();
Line 87: }
Line 88: return instance;
--
To view, visit http://gerrit.ovirt.org/15579
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I87f5a6684701c9d5f603bf79459a3a5b6614b24f
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alexander Wels <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Greg Sheremeta <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
