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

ChangeLog.md

@@ -17,6 +17,13 @@ specific language governing permissions and limitations
 under the License.
 -->
 
+**2023-12-06 Arturo Bernal (abernal AT apache DOT org)**
+
+* _2.12.2-git-12_
+*
+* [JSPWIKI-1178](https://issues.apache.org/jira/browse/JSPWIKI-1178) - Deadlock with Java Virtual Threads
+
+
 **2023-12-04  Juan Pablo Santos (juanpablo AT apache DOT org)**
 
 * _2.12.2-git-11_

jspwiki-api/src/main/java/org/apache/wiki/api/Release.java

@@ -69,7 +69,7 @@ public final class Release {
      *  <p>
      *  If the build identifier is empty, it is not added.
      */
-    public static final String     BUILD         = "11";
+    public static final String     BUILD         = "12";
 
     /**
      *  This is the generic version string you should use when printing out the version.  It is of

jspwiki-event/pom.xml

@@ -62,5 +62,11 @@
       <artifactId>junit-jupiter-engine</artifactId>
       <scope>test</scope>
     </dependency>
+
+    <dependency>
+      <groupId>${project.groupId}</groupId>
+      <artifactId>jspwiki-util</artifactId>
+      <version>${project.version}</version>
+    </dependency>
   </dependencies>
 </project>

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

@@ -21,18 +21,21 @@ Licensed to the Apache Software Foundation (ASF) under one
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.apache.wiki.util.Synchronizer;
 
 import java.lang.ref.WeakReference;
-import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.ConcurrentModificationException;
-import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Set;
 import java.util.TreeSet;
 import java.util.Vector;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.atomic.AtomicBoolean;
+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
@@ -133,14 +136,25 @@ public final class WikiEventManager {
     private static WikiEventListener c_monitor;
 
     /* The Map of client object to WikiEventDelegate. */
-    private final Map< Object, WikiEventDelegate > m_delegates = new HashMap<>();
+    private final Map< Object, WikiEventDelegate > m_delegates = new ConcurrentHashMap<>();
 
     /* The Vector containing any preloaded WikiEventDelegates. */
     private final Vector< WikiEventDelegate > m_preloadCache = new Vector<>();
 
     /* Singleton instance of the WikiEventManager. */
     private static WikiEventManager c_instance;
 
+    /*
+     * Locks used to ensure thread safety when accessing shared resources.
+     * This locks provide 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 instanceLock = new ReentrantLock();
+    private static final ReentrantLock removeWikiEventListenerLock = new ReentrantLock();
+
     /** Constructor for a WikiEventManager. */
     private WikiEventManager() {
         c_instance = this;
@@ -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(instanceLock, () -> {
+                if (c_instance == null) {
+                    c_instance = new WikiEventManager();
+                    // start up any post-instantiation services here
+                }
+            });
         }
         return c_instance;
     }
@@ -238,32 +254,28 @@ public static Set<WikiEventListener> getWikiEventListeners( final Object client
      * @return true if the listener was found and removed.
      */
     public static boolean removeWikiEventListener( final WikiEventListener listener ) {
-        boolean removed = false;
+        final AtomicBoolean removed = new AtomicBoolean(false);
         // 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 ) {
+        Synchronizer.synchronize(removeWikiEventListenerLock, () -> {
             // 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
                 final WikiEventDelegate delegate = entry.getValue();
 
                 // now see if we can remove the listener from the delegate (delegate may be null because this is a weak reference)
                 if( delegate != null && delegate.removeWikiEventListener( listener ) ) {
-                    removed = true; // was removed
+                    removed.set(true); // was removed
                 }
             }
-        }
-        return removed;
+        });
+        return removed.get();
     }
 
     private void removeDelegates() {
-        synchronized( m_delegates ) {
-            m_delegates.clear();
-        }
-        synchronized( m_preloadCache ) {
-            m_preloadCache.clear();
-        }
+        m_delegates.clear();
+        m_preloadCache.clear();
     }
 
     public static void shutdown() {
@@ -314,35 +326,33 @@ private Map< Object, WikiEventDelegate > getDelegates() {
      * @param client   the client Object, or alternately a Class reference
      * @return the WikiEventDelegate.
      */
-    private WikiEventDelegate getDelegateFor( final Object client ) {
-        synchronized( m_delegates ) {
-            if( client == null || client instanceof Class ) { // then preload the cache
-                final WikiEventDelegate delegate = new WikiEventDelegate( client );
-                m_preloadCache.add( delegate );
-                m_delegates.put( client, delegate );
-                return delegate;
-            } else if( !m_preloadCache.isEmpty() ) {
-                // then see if any of the cached delegates match the class of the incoming client
-                for( int i = m_preloadCache.size()-1 ; i >= 0 ; i-- ) { // start with most-recently added
-                    final WikiEventDelegate delegate = m_preloadCache.elementAt( i );
-                    if( delegate.getClientClass() == null || delegate.getClientClass().equals( client.getClass() ) ) {
-                        // we have a hit, so use it, but only on a client we haven't seen before
-                        if( !m_delegates.containsKey( client ) ) {
-                            m_preloadCache.remove( delegate );
-                            m_delegates.put( client, delegate );
-                            return delegate;
-                        }
+    private WikiEventDelegate getDelegateFor(final Object client) {
+        if (client == null || client instanceof Class) { // then preload the cache
+            final WikiEventDelegate delegate = new WikiEventDelegate(client);
+            m_preloadCache.add(delegate);
+            m_delegates.put(client, delegate);
+            return delegate;
+        } else if (!m_preloadCache.isEmpty()) {
+            // then see if any of the cached delegates match the class of the incoming client
+            for (int i = m_preloadCache.size() - 1; i >= 0; i--) { // start with most-recently added
+                final WikiEventDelegate delegate = m_preloadCache.elementAt(i);
+                if (delegate.getClientClass() == null || delegate.getClientClass().equals(client.getClass())) {
+                    // we have a hit, so use it, but only on a client we haven't seen before
+                    if (!m_delegates.containsKey(client)) {
+                        m_preloadCache.remove(delegate);
+                        m_delegates.put(client, delegate);
+                        return delegate;
                     }
                 }
             }
-            // otherwise treat normally...
-            WikiEventDelegate delegate = m_delegates.get( client );
-            if( delegate == null ) {
-                delegate = new WikiEventDelegate( client );
-                m_delegates.put( client, delegate );
-            }
-            return delegate;
         }
+        // otherwise treat normally...
+        WikiEventDelegate delegate = m_delegates.get(client);
+        if (delegate == null) {
+            delegate = new WikiEventDelegate(client);
+            m_delegates.put(client, delegate);
+        }
+        return delegate;
     }
 
 
@@ -358,7 +368,7 @@ private WikiEventDelegate getDelegateFor( final Object client ) {
     private static final class WikiEventDelegate {
 
         /* A list of event listeners for this instance. */
-        private final ArrayList< WeakReference< WikiEventListener > > m_listenerList = new ArrayList<>();
+        private final CopyOnWriteArrayList< WeakReference< WikiEventListener > > m_listenerList = new CopyOnWriteArrayList<>();
         private Class< ? >  m_class;
 
         /**
@@ -386,17 +396,15 @@ 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 ) {
-                final TreeSet< WikiEventListener > set = new TreeSet<>( new WikiEventListenerComparator() );
-                for( final WeakReference< WikiEventListener > wikiEventListenerWeakReference : m_listenerList ) {
-                    final WikiEventListener l = wikiEventListenerWeakReference.get();
-                    if( l != null ) {
-                        set.add( l );
-                    }
+            final TreeSet< WikiEventListener > set = new TreeSet<>( new WikiEventListenerComparator() );
+            for( final WeakReference< WikiEventListener > wikiEventListenerWeakReference : m_listenerList ) {
+                final WikiEventListener l = wikiEventListenerWeakReference.get();
+                if( l != null ) {
+                    set.add( l );
                 }
-
-                return Collections.unmodifiableSet( set );
             }
+
+            return Collections.unmodifiableSet( set );
         }
 
         /**
@@ -406,13 +414,11 @@ 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 ) {
-                final boolean listenerAlreadyContained = m_listenerList.stream()
-                                                                       .map( WeakReference::get )
-                                                                       .anyMatch( ref -> ref == listener );
-                if( !listenerAlreadyContained ) {
-                    return m_listenerList.add( new WeakReference<>( listener ) );
-                }
+            final boolean listenerAlreadyContained = m_listenerList.stream()
+                                                                   .map( WeakReference::get )
+                                                                   .anyMatch( ref -> ref == listener );
+            if( !listenerAlreadyContained ) {
+                return m_listenerList.add( new WeakReference<>( listener ) );
             }
             return false;
         }
@@ -424,26 +430,21 @@ 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 ) {
-                for( final Iterator< WeakReference< WikiEventListener > > i = m_listenerList.iterator(); i.hasNext(); ) {
-                    final WikiEventListener l = i.next().get();
-                    if( l == listener ) {
-                        i.remove();
-                        return true;
-                    }
+            for (final Iterator<WeakReference<WikiEventListener>> i = m_listenerList.iterator(); i.hasNext(); ) {
+                final WikiEventListener l = i.next().get();
+                if (l == listener) {
+                    i.remove();
+                    return true;
                 }
             }
-
             return false;
         }
 
         /**
          *  Returns true if there are one or more listeners registered with this instance.
          */
         public boolean isListening() {
-            synchronized( m_listenerList ) {
-                return !m_listenerList.isEmpty();
-            }
+            return !m_listenerList.isEmpty();
         }
 
         /**
@@ -452,29 +453,26 @@ public boolean isListening() {
         public void fireEvent( final WikiEvent event ) {
             boolean needsCleanup = false;
             try {
-                synchronized( m_listenerList ) {
-                    for( final WeakReference< WikiEventListener > wikiEventListenerWeakReference : m_listenerList ) {
-                        final WikiEventListener listener = wikiEventListenerWeakReference.get();
-                        if( listener != null ) {
-                            listener.actionPerformed( event );
-                        } else {
-                            needsCleanup = true;
-                        }
+                for( final WeakReference< WikiEventListener > wikiEventListenerWeakReference : m_listenerList ) {
+                    final WikiEventListener listener = wikiEventListenerWeakReference.get();
+                    if( listener != null ) {
+                        listener.actionPerformed( event );
+                    } else {
+                        needsCleanup = true;
                     }
+                }
 
-                    //  Remove all such listeners which have expired
-                    if( needsCleanup ) {
-                        for( int i = 0; i < m_listenerList.size(); i++ ) {
-                            final WeakReference< WikiEventListener > w = m_listenerList.get( i );
-                            if( w.get() == null ) {
-                                m_listenerList.remove(i--);
-                            }
+                // Remove all such listeners which have expired
+                if( needsCleanup ) {
+                    for( int i = 0; i < m_listenerList.size(); i++ ) {
+                        final WeakReference< WikiEventListener > w = m_listenerList.get( i );
+                        if( w.get() == null ) {
+                            m_listenerList.remove( i-- );
                         }
                     }
-
                 }
-            } catch( final ConcurrentModificationException e ) {
-                //  We don't die, we just don't do notifications in that case.
+            } catch ( final ConcurrentModificationException e ) {
+                // We don't die, we just don't do notifications in that case.
                 LOG.info( "Concurrent modification of event list; please report this.", e );
             }
         }