[JSPWIKI-1178] Address potential deadlock with JDK 21 Virtual Threads.

Refactored synchronized blocks/methods containing blocking operations to prevent potential deadlocks introduced by the upcoming Virtual Threads feature in JDK 21.

Author: arturobernalg

jspwiki-event/src/main/java/org/apache/wiki/event/WikiEventManager.java

@@ -33,6 +33,7 @@ Licensed to the Apache Software Foundation (ASF) under one
 import java.util.Set;
 import java.util.TreeSet;
 import java.util.Vector;
+import java.util.concurrent.locks.ReentrantLock;
 
 /**
  *  A singleton class that manages the addition and removal of WikiEvent listeners to a event source, as well as the firing of events
@@ -141,6 +142,17 @@ public final class WikiEventManager {
     /* Singleton instance of the WikiEventManager. */
     private static WikiEventManager c_instance;
 
+    /**
+     * A lock used to ensure thread safety when accessing shared resources.
+     * This lock provides more flexibility and capabilities than the intrinsic locking mechanism,
+     * such as the ability to attempt to acquire a lock with a timeout, or to interrupt a thread
+     * waiting to acquire a lock.
+     *
+     * @see java.util.concurrent.locks.ReentrantLock
+     */
+    private static final ReentrantLock lock = new ReentrantLock();
+
+
     /** Constructor for a WikiEventManager. */
     private WikiEventManager() {
         c_instance = this;
@@ -154,10 +166,15 @@ 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) {
+            lock.lock();
+            try {
+                if (c_instance == null) {
+                    c_instance = new WikiEventManager();
+                    // start up any post-instantiation services here
+                }
+            } finally {
+                lock.unlock();
             }
         }
         return c_instance;
@@ -242,7 +259,8 @@ public static boolean removeWikiEventListener( final WikiEventListener listener
         // get the Map.entry object for the entire Map, then check match on entry (listener)
         final WikiEventManager mgr = getInstance();
         final Map< Object, WikiEventDelegate > sources =  Collections.synchronizedMap( mgr.getDelegates() );
-        synchronized( sources ) {
+        lock.lock();
+        try {
             // get an iterator over the Map.Enty objects in the map
             for( final Map.Entry< Object, WikiEventDelegate > entry : sources.entrySet() ) {
                 // the entry value is the delegate
@@ -253,16 +271,24 @@ public static boolean removeWikiEventListener( final WikiEventListener listener
                     removed = true; // was removed
                 }
             }
+        } finally {
+            lock.unlock();
         }
         return removed;
     }
 
     private void removeDelegates() {
-        synchronized( m_delegates ) {
+        lock.lock();
+        try {
             m_delegates.clear();
+        } finally {
+            lock.unlock();
         }
-        synchronized( m_preloadCache ) {
+        lock.lock();
+        try {
             m_preloadCache.clear();
+        } finally {
+            lock.unlock();
         }
     }
 
@@ -315,7 +341,8 @@ private Map< Object, WikiEventDelegate > getDelegates() {
      * @return the WikiEventDelegate.
      */
     private WikiEventDelegate getDelegateFor( final Object client ) {
-        synchronized( m_delegates ) {
+        lock.lock();
+        try {
             if( client == null || client instanceof Class ) { // then preload the cache
                 final WikiEventDelegate delegate = new WikiEventDelegate( client );
                 m_preloadCache.add( delegate );
@@ -342,6 +369,8 @@ private WikiEventDelegate getDelegateFor( final Object client ) {
                 m_delegates.put( client, delegate );
             }
             return delegate;
+        } finally {
+            lock.unlock();
         }
     }
 
@@ -386,7 +415,8 @@ private static final class WikiEventDelegate {
          * @throws java.lang.UnsupportedOperationException  if any attempt is made to modify the Set
          */
         public Set< WikiEventListener > getWikiEventListeners() {
-            synchronized( m_listenerList ) {
+            lock.lock();
+            try {
                 final TreeSet< WikiEventListener > set = new TreeSet<>( new WikiEventListenerComparator() );
                 for( final WeakReference< WikiEventListener > wikiEventListenerWeakReference : m_listenerList ) {
                     final WikiEventListener l = wikiEventListenerWeakReference.get();
@@ -396,6 +426,8 @@ public Set< WikiEventListener > getWikiEventListeners() {
                 }
 
                 return Collections.unmodifiableSet( set );
+            } finally {
+                lock.unlock();
             }
         }
 
@@ -406,13 +438,16 @@ public Set< WikiEventListener > getWikiEventListeners() {
          * @return true if the listener was added (i.e., it was not already in the list and was added)
          */
         public boolean addWikiEventListener( final WikiEventListener listener ) {
-            synchronized( m_listenerList ) {
+            lock.lock();
+            try {
                 final boolean listenerAlreadyContained = m_listenerList.stream()
                                                                        .map( WeakReference::get )
                                                                        .anyMatch( ref -> ref == listener );
                 if( !listenerAlreadyContained ) {
                     return m_listenerList.add( new WeakReference<>( listener ) );
                 }
+            } finally {
+                lock.unlock();
             }
             return false;
         }
@@ -424,14 +459,17 @@ public boolean addWikiEventListener( final WikiEventListener listener ) {
          * @return true if the listener was removed (i.e., it was actually in the list and was removed)
          */
         public boolean removeWikiEventListener( final WikiEventListener listener ) {
-            synchronized( m_listenerList ) {
+            lock.lock();
+            try {
                 for( final Iterator< WeakReference< WikiEventListener > > i = m_listenerList.iterator(); i.hasNext(); ) {
                     final WikiEventListener l = i.next().get();
                     if( l == listener ) {
                         i.remove();
                         return true;
                     }
                 }
+            } finally {
+                lock.unlock();
             }
 
             return false;
@@ -441,8 +479,11 @@ public boolean removeWikiEventListener( final WikiEventListener listener ) {
          *  Returns true if there are one or more listeners registered with this instance.
          */
         public boolean isListening() {
-            synchronized( m_listenerList ) {
+            lock.lock();
+            try {
                 return !m_listenerList.isEmpty();
+            } finally {
+                lock.unlock();
             }
         }
 
@@ -452,7 +493,8 @@ public boolean isListening() {
         public void fireEvent( final WikiEvent event ) {
             boolean needsCleanup = false;
             try {
-                synchronized( m_listenerList ) {
+                lock.lock();
+                try {
                     for( final WeakReference< WikiEventListener > wikiEventListenerWeakReference : m_listenerList ) {
                         final WikiEventListener listener = wikiEventListenerWeakReference.get();
                         if( listener != null ) {
@@ -472,6 +514,8 @@ public void fireEvent( final WikiEvent event ) {
                         }
                     }
 
+                } finally {
+                    lock.unlock();
                 }
             } catch( final ConcurrentModificationException e ) {
                 //  We don't die, we just don't do notifications in that case.

jspwiki-kendra-searchprovider/src/main/java/org/apache/wiki/search/kendra/KendraSearchProvider.java

@@ -55,6 +55,7 @@ Licensed to the Apache Software Foundation (ASF) under one
 import java.nio.ByteBuffer;
 import java.nio.charset.StandardCharsets;
 import java.util.*;
+import java.util.concurrent.locks.ReentrantLock;
 
 import static java.lang.String.format;
 
@@ -87,7 +88,18 @@ public class KendraSearchProvider implements SearchProvider {
     private static final String PROP_KENDRA_INDEXDELAY = "jspwiki.kendra.indexdelay";
     private static final String PROP_KENDRA_INITIALDELAY = "jspwiki.kendra.initialdelay";
 
+    /**
+     * A lock used to ensure thread safety when accessing shared resources.
+     * This lock provides more flexibility and capabilities than the intrinsic locking mechanism,
+     * such as the ability to attempt to acquire a lock with a timeout, or to interrupt a thread
+     * waiting to acquire a lock.
+     *
+     * @see java.util.concurrent.locks.ReentrantLock
+     */
+    private final ReentrantLock lock;
+
     public KendraSearchProvider() {
+         lock = new ReentrantLock();
     }
 
     /**
@@ -339,14 +351,17 @@ private void doPartialReindex() {
         }
         LOG.debug( "Indexing updated pages. Please wait ..." );
         final String executionId = startExecution();
-        synchronized ( updates ) {
+        lock.lock();
+        try {
             try {
                 while ( updates.size() > 0 ) {
                     indexOnePage( updates.remove( 0 ), executionId );
                 }
             } finally {
                 stopExecution();
             }
+        } finally {
+            lock.unlock();
         }
     }
 

jspwiki-main/src/main/java/org/apache/wiki/WatchDog.java

@@ -28,6 +28,7 @@ Licensed to the Apache Software Foundation (ASF) under one
 import java.util.Set;
 import java.util.Stack;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.locks.ReentrantLock;
 
 
 /**
@@ -57,6 +58,16 @@ public final class WatchDog {
     private static final Map< Integer, WeakReference< WatchDog > > c_kennel = new ConcurrentHashMap<>();
     private static WikiBackgroundThread c_watcherThread;
 
+    /**
+     * A lock used to ensure thread safety when accessing shared resources.
+     * This lock provides more flexibility and capabilities than the intrinsic locking mechanism,
+     * such as the ability to attempt to acquire a lock with a timeout, or to interrupt a thread
+     * waiting to acquire a lock.
+     *
+     * @see java.util.concurrent.locks.ReentrantLock
+     */
+    private final ReentrantLock lock;
+
     /**
      *  Returns the current watchdog for the current thread. This is the preferred method of getting you a Watchdog, since it
      *  keeps an internal list of Watchdogs for you so that there won't be more than one watchdog per thread.
@@ -92,11 +103,16 @@ public WatchDog( final Engine engine, final Watchable watch ) {
         m_engine    = engine;
         m_watchable = watch;
 
-        synchronized( WatchDog.class ) {
+        lock = new ReentrantLock();
+
+        lock.lock();
+        try {
             if( c_watcherThread == null ) {
                 c_watcherThread = new WatchDogThread( engine );
                 c_watcherThread.start();
             }
+        } finally {
+            lock.unlock();
         }
     }
 
@@ -136,25 +152,31 @@ private static void scrub() {
      *  Can be used to enable the WatchDog.  Will cause a new Thread to be created, if none was existing previously.
      */
     public void enable() {
-        synchronized( WatchDog.class ) {
+        lock.lock();
+        try {
             if( !m_enabled ) {
                 m_enabled = true;
                 c_watcherThread = new WatchDogThread( m_engine );
                 c_watcherThread.start();
             }
+        } finally {
+            lock.unlock();
         }
     }
 
     /**
      *  Is used to disable a WatchDog.  The watchdog thread is shut down and resources released.
      */
     public void disable() {
-        synchronized( WatchDog.class ) {
+        lock.lock();
+        try {
             if( m_enabled ) {
                 m_enabled = false;
                 c_watcherThread.shutdown();
                 c_watcherThread = null;
             }
+        } finally {
+            lock.unlock();
         }
     }
 
@@ -186,9 +208,12 @@ public void enterState( final String state ) {
      */
     public void enterState( final String state, final int expectedCompletionTime ) {
         LOG.debug(  "{}: Entering state {}, expected completion in {} s", m_watchable.getName(), state, expectedCompletionTime );
-        synchronized( m_stateStack ) {
+        lock.lock();
+        try {
             final State st = new State( state, expectedCompletionTime );
             m_stateStack.push( st );
+        } finally {
+            lock.unlock();
         }
     }
 
@@ -208,7 +233,8 @@ public void exitState() {
      */
     public void exitState( final String state ) {
         if( !m_stateStack.empty() ) {
-            synchronized( m_stateStack ) {
+            lock.lock();
+            try {
                 final State st = m_stateStack.peek();
                 if( state == null || st.getState().equals( state ) ) {
                     m_stateStack.pop();
@@ -218,6 +244,8 @@ public void exitState( final String state ) {
                     // FIXME: should actually go and fix things for that
                     LOG.error( "exitState() called before enterState()" );
                 }
+            } finally {
+                lock.unlock();
             }
         } else {
             LOG.warn( "Stack for " + m_watchable.getName() + " is empty!" );
@@ -244,8 +272,8 @@ public boolean isWatchableAlive() {
 
     private void check() {
         LOG.debug( "Checking watchdog '{}'", m_watchable.getName() );
-
-        synchronized( m_stateStack ) {
+        lock.lock();
+        try {
             if( !m_stateStack.empty() ) {
                 final State st = m_stateStack.peek();
                 final long now = System.currentTimeMillis();
@@ -261,6 +289,8 @@ private void check() {
             } else {
                 LOG.warn( "Stack for " + m_watchable.getName() + " is empty!" );
             }
+        } finally {
+            lock.unlock();
         }
     }
 
@@ -302,14 +332,17 @@ private void dumpStackTraceForWatchable() {
      */
     @Override
     public String toString() {
-        synchronized( m_stateStack ) {
+        lock.lock();
+        try {
             String state = "Idle";
 
             if( !m_stateStack.empty() ) {
                 final State st = m_stateStack.peek();
                 state = st.getState();
             }
             return "WatchDog state=" + state;
+        } finally {
+            lock.unlock();
         }
     }