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

Reply via email to