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 <aw...@redhat.com>
Gerrit-Reviewer: Alexander Wels <aw...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Einav Cohen <eco...@redhat.com>
Gerrit-Reviewer: Greg Sheremeta <gsher...@redhat.com>
Gerrit-Reviewer: Michael Pasternak <mpast...@redhat.com>
Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to