Hi, My use of the synchronized block in WikiEventManager was based on a section "Use Double-Checked Locking to Reduce Synchronization" of Craig Larman and Rhett Guthrie's book "Java 2: Performance and Idiom Guide"* about the use of a synchronized block on singleton classes protected by a pair of null checks.
I have been successfully using this approach in my own software designs since the 90s, quoting from page 100: "Consider the example of a 'Singleton' method implementing the Singleton pattern [GHJV 1995]. This is likely to be a high-use method, but the critical section is in the rarely entered lazy initialization block. It is not desirable to synchronize the method, when only a very rarely entered block contains the critical section for which the synchronization was required. Synchronizing the entire method adds unnecessary overhead for all method invocations, and reduces concurrency." What I take from this is somewhat of a cautionary advisory not to use this new lock-based approach everywhere in JSPWiki, that a simple double- null check was a means to avoid a synchronized block's overhead. Use of locks certainly does have its place -- particularly where the use of the lock spans methods -- but the creation of a lock object, the necessity for an additional utility method call, the potential for performance hits, and the additional possibility of exceptions and their handling when locks cannot be obtained, all this additional complexity might suggest that at least for this particular use in lazily creating singletons the synchronized keyword probably makes the most sense. A typical existing usage of the double null check for singletons: /** * Returns the single instance of this class. * <p> * If the singleton has already been generated it is returned. * </p> */ public static IdGenerator getInstance() { if ( instance == null ) { synchronized (IdGenerator.class) { // see larman/guthrie p.100 if ( instance == null ) { IdGenerator idg = new IdGenerator(IdType.BASE64); return idg; } } } return instance; } While use of the proposed Synchronizer method might appear to be a simplification, public static IdGenerator getInstance() { if ( instance == null ) { final ReentrantLock lock = new ReentrantLock(); Synchronizer.synchronize(lock, () -> { IdGenerator idg = new IdGenerator(IdType.BASE64); return idg; } lock.unlock(); } return instance; } Under the covers the replacement for this would be akin to: public static IdGenerator getInstance() { if ( instance == null ) { final ReentrantLock lock = new ReentrantLock(); try { if ( lock.tryLock(20, TimeUnit.MILLISECONDS) ) { IdGenerator idg = new IdGenerator(IdType.BASE64); return idg; } else { throw new RuntimeException("unable to obtain lock."); } } catch ( InterruptedException e ) { throw new RuntimeException("the current thread was interrupted: " + e.getMessage()); } finally { lock.unlock(); } } return instance; } Though I'm entirely willing to entertain discussion to the contrary as I'm possibly not understanding the proposal or its implementation. I'm also sure that a book published in 1999 for Java 2 may have been superceded by more current ideas about how to handle high performance concurrent applications, and indeed, if anyone knows of a good book on the subject that's up to date with JDK 21 I'd be interested in knowing about it. Murray * https://www.abebooks.com/9780130142603/Java-2-Performance-Idiom-Guide-0130142603/plp On 11/10/23 22:52, juanpablo-santos (via GitHub) wrote:
juanpablo-santos commented on code in PR #307: URL: https://github.com/apache/jspwiki/pull/307#discussion_r1354460996
[...]
########## jspwiki-event/src/main/java/org/apache/wiki/event/WikiEventManager.java: ########## @@ -154,11 +168,13 @@ private WikiEventManager() { * @return A shared instance of the WikiEventManager */ public static WikiEventManager getInstance() { - if( c_instance == null ) { - synchronized( WikiEventManager.class ) { - return new WikiEventManager(); - // start up any post-instantiation services here - } + if (c_instance == null) { + Synchronizer.synchronize(lock, () -> {
........................................................................... Murray Altheim <murray18 at altheim dot com> = = === http://www.altheim.com/murray/ === === = = === In the evening The rice leaves in the garden Rustle in the autumn wind That blows through my reed hut. -- Minamoto no Tsunenobu