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