From 026ba283ba3a700bf4e2c9feba6eb9535f34bc5b Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Tue, 10 Oct 2023 09:04:23 +0200 Subject: [PATCH] Use custom Synchronizer class to manage ReentrantLock synchronization --- jspwiki-event/pom.xml | 5 + .../apache/wiki/event/WikiEventManager.java | 148 +++++++---------- .../search/kendra/KendraSearchProvider.java | 17 +- .../main/java/org/apache/wiki/WatchDog.java | 54 ++----- .../main/java/org/apache/wiki/WikiEngine.java | 49 ++---- .../auth/DefaultAuthenticationManager.java | 15 +- .../auth/DefaultAuthorizationManager.java | 15 +- .../apache/wiki/auth/DefaultUserManager.java | 15 +- .../org/apache/wiki/auth/SessionMonitor.java | 45 ++---- .../apache/wiki/auth/acl/AclEntryImpl.java | 31 +--- .../org/apache/wiki/auth/acl/AclImpl.java | 15 +- .../auth/authorize/DefaultGroupManager.java | 50 ++---- .../org/apache/wiki/auth/authorize/Group.java | 94 ++++------- .../CookieAuthenticationLoginModule.java | 8 +- .../auth/permissions/PermissionFactory.java | 57 +++---- .../wiki/auth/user/XMLUserDatabase.java | 102 ++++++------ .../wiki/diff/ContextualDiffProvider.java | 8 +- .../apache/wiki/filters/PageEventFilter.java | 15 +- .../org/apache/wiki/filters/SpamFilter.java | 15 +- .../apache/wiki/plugin/BugReportHandler.java | 9 +- .../apache/wiki/plugin/PageViewPlugin.java | 44 ++--- .../providers/CachingAttachmentProvider.java | 15 +- .../wiki/providers/CachingProvider.java | 43 ++--- .../providers/VersioningFileProvider.java | 41 ++--- .../references/DefaultReferenceManager.java | 81 +++++----- .../apache/wiki/rss/DefaultRSSGenerator.java | 15 +- .../wiki/search/LuceneSearchProvider.java | 29 ++-- .../ui/admin/beans/SearchManagerBean.java | 8 +- .../apache/wiki/workflow/AbstractStep.java | 36 ++--- .../org/apache/wiki/workflow/Decision.java | 8 +- .../apache/wiki/workflow/DecisionQueue.java | 22 +-- .../wiki/workflow/DefaultWorkflowManager.java | 15 +- .../java/org/apache/wiki/workflow/Task.java | 16 +- .../org/apache/wiki/workflow/Workflow.java | 71 +++------ .../apache/wiki/util/CommentedProperties.java | 56 ++----- .../org/apache/wiki/util/Synchronizer.java | 150 ++++++++++++++++++ pom.xml | 7 + 37 files changed, 610 insertions(+), 814 deletions(-) create mode 100644 jspwiki-util/src/main/java/org/apache/wiki/util/Synchronizer.java diff --git a/jspwiki-event/pom.xml b/jspwiki-event/pom.xml index 2066adbfa0..512418f6e1 100644 --- a/jspwiki-event/pom.xml +++ b/jspwiki-event/pom.xml @@ -62,5 +62,10 @@ junit-jupiter-engine test + + + org.apache.jspwiki + jspwiki-util + diff --git a/jspwiki-event/src/main/java/org/apache/wiki/event/WikiEventManager.java b/jspwiki-event/src/main/java/org/apache/wiki/event/WikiEventManager.java index 0aedc2659a..1d9dad159f 100644 --- a/jspwiki-event/src/main/java/org/apache/wiki/event/WikiEventManager.java +++ b/jspwiki-event/src/main/java/org/apache/wiki/event/WikiEventManager.java @@ -21,6 +21,7 @@ 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; @@ -33,6 +34,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.atomic.AtomicBoolean; import java.util.concurrent.locks.ReentrantLock; /** @@ -167,15 +169,12 @@ private WikiEventManager() { */ public static WikiEventManager getInstance() { if (c_instance == null) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { if (c_instance == null) { c_instance = new WikiEventManager(); // start up any post-instantiation services here } - } finally { - lock.unlock(); - } + }); } return c_instance; } @@ -255,12 +254,11 @@ public static Set 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() ); - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { // 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 @@ -268,28 +266,16 @@ public static boolean removeWikiEventListener( final WikiEventListener listener // 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 } } - } finally { - lock.unlock(); - } - return removed; + }); + return removed.get(); } private void removeDelegates() { - lock.lock(); - try { - m_delegates.clear(); - } finally { - lock.unlock(); - } - lock.lock(); - try { - m_preloadCache.clear(); - } finally { - lock.unlock(); - } + Synchronizer.synchronize(lock, m_delegates::clear); + Synchronizer.synchronize(lock, m_preloadCache::clear); } public static void shutdown() { @@ -340,38 +326,35 @@ private Map< Object, WikiEventDelegate > getDelegates() { * @param client the client Object, or alternately a Class reference * @return the WikiEventDelegate. */ - private WikiEventDelegate getDelegateFor( final Object client ) { - lock.lock(); - try { - 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 ); + private WikiEventDelegate getDelegateFor(final Object client) { + return Synchronizer.synchronize(lock, () -> { + 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() ) { + } 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() ) ) { + 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 ); + 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 ); + WikiEventDelegate delegate = m_delegates.get(client); + if (delegate == null) { + delegate = new WikiEventDelegate(client); + m_delegates.put(client, delegate); } return delegate; - } finally { - lock.unlock(); - } + }); } @@ -415,8 +398,7 @@ private static final class WikiEventDelegate { * @throws java.lang.UnsupportedOperationException if any attempt is made to modify the Set */ public Set< WikiEventListener > getWikiEventListeners() { - lock.lock(); - try { + return Synchronizer.synchronize(lock, () -> { final TreeSet< WikiEventListener > set = new TreeSet<>( new WikiEventListenerComparator() ); for( final WeakReference< WikiEventListener > wikiEventListenerWeakReference : m_listenerList ) { final WikiEventListener l = wikiEventListenerWeakReference.get(); @@ -426,9 +408,7 @@ public Set< WikiEventListener > getWikiEventListeners() { } return Collections.unmodifiableSet( set ); - } finally { - lock.unlock(); - } + }); } /** @@ -438,18 +418,15 @@ 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 ) { - lock.lock(); - try { + return Synchronizer.synchronize(lock, () -> { 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; + return false; + }); } /** @@ -459,67 +436,54 @@ 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 ) { - lock.lock(); - try { - for( final Iterator< WeakReference< WikiEventListener > > i = m_listenerList.iterator(); i.hasNext(); ) { + return Synchronizer.synchronize(lock, () -> { + for (final Iterator> i = m_listenerList.iterator(); i.hasNext(); ) { final WikiEventListener l = i.next().get(); - if( l == listener ) { + if (l == listener) { i.remove(); return true; } } - } finally { - lock.unlock(); - } - - return false; + return false; + }); } /** * Returns true if there are one or more listeners registered with this instance. */ public boolean isListening() { - lock.lock(); - try { - return !m_listenerList.isEmpty(); - } finally { - lock.unlock(); - } + return Synchronizer.synchronize(lock, () -> !m_listenerList.isEmpty()); } /** * Notify all listeners having a registered interest in change events of the supplied WikiEvent. */ - public void fireEvent( final WikiEvent event ) { - boolean needsCleanup = false; + public void fireEvent(final WikiEvent event) { + final AtomicBoolean needsCleanup = new AtomicBoolean(false); try { - lock.lock(); - try { - for( final WeakReference< WikiEventListener > wikiEventListenerWeakReference : m_listenerList ) { + Synchronizer.synchronize(lock, () -> { + for (final WeakReference wikiEventListenerWeakReference : m_listenerList) { final WikiEventListener listener = wikiEventListenerWeakReference.get(); - if( listener != null ) { - listener.actionPerformed( event ); + if (listener != null) { + listener.actionPerformed(event); } else { - needsCleanup = true; + needsCleanup.set(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 ) { + // Remove all such listeners which have expired + if (needsCleanup.get()) { + for (int i = 0; i < m_listenerList.size(); i++) { + final WeakReference w = m_listenerList.get(i); + if (w.get() == null) { m_listenerList.remove(i--); } } } - - } finally { - lock.unlock(); - } - } 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 ); + }); + } 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); } } } diff --git a/jspwiki-kendra-searchprovider/src/main/java/org/apache/wiki/search/kendra/KendraSearchProvider.java b/jspwiki-kendra-searchprovider/src/main/java/org/apache/wiki/search/kendra/KendraSearchProvider.java index 5772e09f8d..bb4c643f60 100644 --- a/jspwiki-kendra-searchprovider/src/main/java/org/apache/wiki/search/kendra/KendraSearchProvider.java +++ b/jspwiki-kendra-searchprovider/src/main/java/org/apache/wiki/search/kendra/KendraSearchProvider.java @@ -46,6 +46,7 @@ Licensed to the Apache Software Foundation (ASF) under one import org.apache.wiki.auth.permissions.PagePermission; import org.apache.wiki.pages.PageManager; import org.apache.wiki.search.SearchProvider; +import org.apache.wiki.util.Synchronizer; import org.apache.wiki.util.TextUtil; import java.io.IOException; @@ -346,23 +347,21 @@ private void doFullReindex() throws IOException { * index pages that have been modified */ private void doPartialReindex() { - if ( updates.isEmpty() ) { + if (updates.isEmpty()) { return; } - LOG.debug( "Indexing updated pages. Please wait ..." ); + LOG.debug("Indexing updated pages. Please wait ..."); final String executionId = startExecution(); - lock.lock(); - try { + + Synchronizer.synchronize(lock, () -> { try { - while ( updates.size() > 0 ) { - indexOnePage( updates.remove( 0 ), executionId ); + while (!updates.isEmpty()) { + indexOnePage(updates.remove(0), executionId); } } finally { stopExecution(); } - } finally { - lock.unlock(); - } + }); } /** diff --git a/jspwiki-main/src/main/java/org/apache/wiki/WatchDog.java b/jspwiki-main/src/main/java/org/apache/wiki/WatchDog.java index 820ece30a4..cc36298bbc 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/WatchDog.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/WatchDog.java @@ -21,6 +21,7 @@ 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.api.core.Engine; +import org.apache.wiki.util.Synchronizer; import java.lang.ref.WeakReference; import java.util.Iterator; @@ -105,15 +106,12 @@ public WatchDog( final Engine engine, final Watchable watch ) { lock = new ReentrantLock(); - lock.lock(); - try { - if( c_watcherThread == null ) { - c_watcherThread = new WatchDogThread( engine ); + Synchronizer.synchronize(lock, () -> { + if (c_watcherThread == null) { + c_watcherThread = new WatchDogThread(engine); c_watcherThread.start(); } - } finally { - lock.unlock(); - } + }); } /** @@ -152,32 +150,26 @@ 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() { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { 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() { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { if( m_enabled ) { m_enabled = false; c_watcherThread.shutdown(); c_watcherThread = null; } - } finally { - lock.unlock(); - } + }); } /** @@ -208,13 +200,10 @@ 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 ); - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { final State st = new State( state, expectedCompletionTime ); m_stateStack.push( st ); - } finally { - lock.unlock(); - } + }); } /** @@ -233,8 +222,7 @@ public void exitState() { */ public void exitState( final String state ) { if( !m_stateStack.empty() ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { final State st = m_stateStack.peek(); if( state == null || st.getState().equals( state ) ) { m_stateStack.pop(); @@ -244,9 +232,7 @@ 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!" ); } @@ -272,8 +258,7 @@ public boolean isWatchableAlive() { private void check() { LOG.debug( "Checking watchdog '{}'", m_watchable.getName() ); - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { if( !m_stateStack.empty() ) { final State st = m_stateStack.peek(); final long now = System.currentTimeMillis(); @@ -289,9 +274,7 @@ private void check() { } else { LOG.warn( "Stack for " + m_watchable.getName() + " is empty!" ); } - } finally { - lock.unlock(); - } + }); } /** @@ -332,8 +315,7 @@ private void dumpStackTraceForWatchable() { */ @Override public String toString() { - lock.lock(); - try { + return Synchronizer.synchronize(lock, () -> { String state = "Idle"; if( !m_stateStack.empty() ) { @@ -341,9 +323,7 @@ public String toString() { state = st.getState(); } return "WatchDog state=" + state; - } finally { - lock.unlock(); - } + }); } /** diff --git a/jspwiki-main/src/main/java/org/apache/wiki/WikiEngine.java b/jspwiki-main/src/main/java/org/apache/wiki/WikiEngine.java index 9895796b16..1c8bd1fdc0 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/WikiEngine.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/WikiEngine.java @@ -57,6 +57,7 @@ Licensed to the Apache Software Foundation (ASF) under one import org.apache.wiki.url.URLConstructor; import org.apache.wiki.util.ClassUtil; import org.apache.wiki.util.PropertyReader; +import org.apache.wiki.util.Synchronizer; import org.apache.wiki.util.TextUtil; import org.apache.wiki.variables.VariableManager; import org.apache.wiki.workflow.WorkflowManager; @@ -81,6 +82,7 @@ Licensed to the Apache Software Foundation (ASF) under one import java.util.Properties; import java.util.TimeZone; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.locks.ReentrantLock; import java.util.stream.Collectors; @@ -158,12 +160,7 @@ public class WikiEngine implements Engine { * @throws InternalWikiException in case something fails. This is a RuntimeException, so be prepared for it. */ public static WikiEngine getInstance( final ServletConfig config ) throws InternalWikiException { - lock.lock(); - try { - return getInstance( config.getServletContext(), null ); - } finally { - lock.unlock(); - } + return Synchronizer.synchronize(lock, () -> getInstance(config.getServletContext(), null)); } /** @@ -177,12 +174,7 @@ public static WikiEngine getInstance( final ServletConfig config ) throws Intern * @return One well-behaving WikiEngine instance. */ public static WikiEngine getInstance( final ServletConfig config, final Properties props ) { - lock.lock(); - try { - return getInstance( config.getServletContext(), props ); - } finally { - lock.unlock(); - } + return Synchronizer.synchronize(lock, () -> getInstance( config.getServletContext(), props )); } /** @@ -194,22 +186,21 @@ public static WikiEngine getInstance( final ServletConfig config, final Properti * @throws InternalWikiException If the WikiEngine instantiation fails. */ public static WikiEngine getInstance(final ServletContext context, Properties props) throws InternalWikiException { - lock.lock(); - WikiEngine engine; - try { - engine = (WikiEngine) context.getAttribute(ATTR_WIKIENGINE); + final AtomicReference propsRef = new AtomicReference<>(props); + return Synchronizer.synchronize(lock, () -> { + WikiEngine engine = (WikiEngine) context.getAttribute(ATTR_WIKIENGINE); if (engine == null) { final String appid = Integer.toString(context.hashCode()); context.log(" Assigning new engine to " + appid); try { - if (props == null) { - props = PropertyReader.loadWebAppProps(context); + if (propsRef.get() == null) { + propsRef.set(PropertyReader.loadWebAppProps(context)); } engine = new WikiEngine(context, appid); try { // Note: May be null, if JSPWiki has been deployed in a WAR file. - engine.start(props); + engine.start(propsRef.get()); LOG.info("Root path for this Wiki is: '{}'", engine.getRootPath()); } catch (final Exception e) { final String msg = Release.APPNAME + ": Unable to load and setup properties from jspwiki.properties. " + e.getMessage(); @@ -224,10 +215,8 @@ public static WikiEngine getInstance(final ServletContext context, Properties pr throw new InternalWikiException("No wiki engine, check logs.", e); } } - } finally { - lock.unlock(); - } - return engine; + return engine; + }); } /** @@ -971,12 +960,9 @@ public InternationalizationManager getInternationalizationManager() { /** {@inheritDoc} */ @Override public final void addWikiEventListener( final WikiEventListener listener ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { WikiEventManager.addWikiEventListener( this, listener ); - } finally { - lock.unlock(); - } + }); } /** @@ -984,12 +970,9 @@ public final void addWikiEventListener( final WikiEventListener listener ) { */ @Override public final void removeWikiEventListener(final WikiEventListener listener) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { WikiEventManager.removeWikiEventListener(this, listener); - } finally { - lock.unlock(); - } + }); } /** diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultAuthenticationManager.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultAuthenticationManager.java index 538363121e..1737fd3845 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultAuthenticationManager.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultAuthenticationManager.java @@ -37,6 +37,7 @@ Licensed to the Apache Software Foundation (ASF) under one import org.apache.wiki.event.WikiEventManager; import org.apache.wiki.event.WikiSecurityEvent; import org.apache.wiki.util.ClassUtil; +import org.apache.wiki.util.Synchronizer; import org.apache.wiki.util.TextUtil; import org.apache.wiki.util.TimedCounterList; @@ -369,12 +370,9 @@ public Set< Principal > doJAASLogin( final Class< ? extends LoginModule > clazz, */ @Override public void addWikiEventListener( final WikiEventListener listener ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { WikiEventManager.addWikiEventListener( this, listener ); - } finally { - lock.unlock(); - } + }); } /** @@ -382,12 +380,9 @@ public void addWikiEventListener( final WikiEventListener listener ) { */ @Override public void removeWikiEventListener( final WikiEventListener listener ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { WikiEventManager.removeWikiEventListener( this, listener ); - } finally { - lock.unlock(); - } + }); } /** diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultAuthorizationManager.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultAuthorizationManager.java index f6747cbb18..79a72ce8a0 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultAuthorizationManager.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultAuthorizationManager.java @@ -44,6 +44,7 @@ Licensed to the Apache Software Foundation (ASF) under one import org.apache.wiki.pages.PageManager; import org.apache.wiki.preferences.Preferences; import org.apache.wiki.util.ClassUtil; +import org.apache.wiki.util.Synchronizer; import org.freshcookies.security.policy.LocalPolicy; import javax.servlet.http.HttpServletResponse; @@ -386,23 +387,17 @@ public Principal resolvePrincipal( final String name ) { /** {@inheritDoc} */ @Override public void addWikiEventListener( final WikiEventListener listener ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { WikiEventManager.addWikiEventListener( this, listener ); - } finally { - lock.unlock(); - } + }); } /** {@inheritDoc} */ @Override public void removeWikiEventListener( final WikiEventListener listener ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { WikiEventManager.removeWikiEventListener( this, listener ); - } finally { - lock.unlock(); - } + }); } } diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultUserManager.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultUserManager.java index 32dde21a35..74c8ded51c 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultUserManager.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultUserManager.java @@ -47,6 +47,7 @@ Licensed to the Apache Software Foundation (ASF) under one import org.apache.wiki.tasks.TasksManager; import org.apache.wiki.ui.InputValidator; import org.apache.wiki.util.ClassUtil; +import org.apache.wiki.util.Synchronizer; import org.apache.wiki.util.TextUtil; import org.apache.wiki.workflow.Decision; import org.apache.wiki.workflow.DecisionRequiredException; @@ -423,12 +424,9 @@ public Principal[] listWikiNames() throws WikiSecurityException { * @param listener the event listener */ @Override public void addWikiEventListener( final WikiEventListener listener ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { WikiEventManager.addWikiEventListener( this, listener ); - } finally { - lock.unlock(); - } + }); } /** @@ -437,12 +435,9 @@ public Principal[] listWikiNames() throws WikiSecurityException { * @param listener the event listener */ @Override public void removeWikiEventListener( final WikiEventListener listener ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { WikiEventManager.removeWikiEventListener( this, listener ); - } finally { - lock.unlock(); - } + }); } /** diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/SessionMonitor.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/SessionMonitor.java index 638e246ae1..38b4baf5c2 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/auth/SessionMonitor.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/SessionMonitor.java @@ -27,6 +27,7 @@ Licensed to the Apache Software Foundation (ASF) under one import org.apache.wiki.event.WikiEventListener; import org.apache.wiki.event.WikiEventManager; import org.apache.wiki.event.WikiSecurityEvent; +import org.apache.wiki.util.Synchronizer; import org.apache.wiki.util.comparators.PrincipalComparator; import javax.servlet.http.HttpServletRequest; @@ -185,12 +186,9 @@ public final Session find( final String sessionId ) { private Session createGuestSessionFor( final String sessionId ) { LOG.debug( "Session for session ID={}... not found. Creating guestSession()", sessionId ); final Session wikiSession = Wiki.session().guest( m_engine ); - lock.lock(); - try { - m_sessions.put( sessionId, wikiSession ); - } finally { - lock.unlock(); - } + Synchronizer.synchronize(lock, () -> { + m_sessions.put(sessionId, wikiSession); + }); return wikiSession; } @@ -215,12 +213,9 @@ public final void remove( final HttpSession session ) { if( session == null ) { throw new IllegalArgumentException( "Session cannot be null." ); } - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { m_sessions.remove( session.getId() ); - } finally { - lock.unlock(); - } + }); } /** @@ -242,15 +237,11 @@ public final int sessions() * @return the array of user principals */ public final Principal[] userPrincipals() { - final Collection principals; - lock.lock(); - try { - principals = m_sessions.values().stream().map(Session::getUserPrincipal).collect(Collectors.toList()); - } finally { - lock.unlock(); - } - final Principal[] p = principals.toArray( new Principal[0] ); - Arrays.sort( p, m_comparator ); + final Collection principals = Synchronizer.synchronize(lock, () -> + m_sessions.values().stream().map(Session::getUserPrincipal).collect(Collectors.toList())); + + final Principal[] p = principals.toArray(new Principal[0]); + Arrays.sort(p, m_comparator); return p; } @@ -261,12 +252,9 @@ public final Principal[] userPrincipals() { * @since 2.4.75 */ public final void addWikiEventListener( final WikiEventListener listener ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { WikiEventManager.addWikiEventListener( this, listener ); - } finally { - lock.unlock(); - } + }); } /** @@ -276,12 +264,9 @@ public final void addWikiEventListener( final WikiEventListener listener ) { * @since 2.4.75 */ public final void removeWikiEventListener(final WikiEventListener listener) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { WikiEventManager.removeWikiEventListener(this, listener); - } finally { - lock.unlock(); - } + }); } /** diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/acl/AclEntryImpl.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/acl/AclEntryImpl.java index befac2c77e..3a2d6d6d0b 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/auth/acl/AclEntryImpl.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/acl/AclEntryImpl.java @@ -20,6 +20,7 @@ Licensed to the Apache Software Foundation (ASF) under one import org.apache.wiki.auth.permissions.PagePermission; import org.apache.wiki.event.WikiEventManager; +import org.apache.wiki.util.Synchronizer; import java.io.Serializable; import java.security.Permission; @@ -68,17 +69,13 @@ public AclEntryImpl() { */ @Override public boolean addPermission(final Permission permission) { - lock.lock(); - try { + return Synchronizer.synchronize(lock, () -> { if (permission instanceof PagePermission && findPermission(permission) == null) { m_permissions.add(permission); return true; } - return false; - } finally { - lock.unlock(); - } + }); } /** @@ -100,12 +97,7 @@ public boolean checkPermission(final Permission permission ) { */ @Override public Principal getPrincipal() { - lock.lock(); - try { - return m_principal; - } finally { - lock.unlock(); - } + return Synchronizer.synchronize(lock, () -> m_principal); } /** @@ -126,18 +118,14 @@ public Enumeration< Permission > permissions() { */ @Override public boolean removePermission(final Permission permission ) { - lock.lock(); - try { + return Synchronizer.synchronize(lock, () -> { final Permission p = findPermission(permission); if (p != null) { m_permissions.remove(p); return true; } - return false; - } finally { - lock.unlock(); - } + }); } /** @@ -150,16 +138,13 @@ public boolean removePermission(final Permission permission ) { */ @Override public boolean setPrincipal(final Principal user) { - lock.lock(); - try { + return Synchronizer.synchronize(lock, () -> { if (m_principal != null || user == null) { return false; } m_principal = user; return true; - } finally { - lock.unlock(); - } + }); } /** diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/acl/AclImpl.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/acl/AclImpl.java index 3eb2ddf729..54af42c608 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/auth/acl/AclImpl.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/acl/AclImpl.java @@ -19,6 +19,7 @@ Licensed to the Apache Software Foundation (ASF) under one package org.apache.wiki.auth.acl; import org.apache.wiki.api.core.AclEntry; +import org.apache.wiki.util.Synchronizer; import java.io.Serializable; import java.security.Permission; @@ -99,8 +100,7 @@ private boolean hasEntry( final AclEntry entry ) { /** {@inheritDoc} */ @Override public boolean addEntry( final AclEntry entry ) { - lock.lock(); - try { + return Synchronizer.synchronize(lock, () -> { if( entry.getPrincipal() == null ) { throw new IllegalArgumentException( "Entry principal cannot be null" ); } @@ -112,20 +112,13 @@ public boolean addEntry( final AclEntry entry ) { m_entries.add( entry ); return true; - } finally { - lock.unlock(); - } + }); } /** {@inheritDoc} */ @Override public boolean removeEntry( final AclEntry entry ) { - lock.lock(); - try { - return m_entries.remove( entry ); - } finally { - lock.unlock(); - } + return Synchronizer.synchronize(lock, () -> m_entries.remove( entry )); } /** {@inheritDoc} */ diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/authorize/DefaultGroupManager.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/authorize/DefaultGroupManager.java index eefc53bec0..b7d1578e8b 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/auth/authorize/DefaultGroupManager.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/authorize/DefaultGroupManager.java @@ -40,6 +40,7 @@ Licensed to the Apache Software Foundation (ASF) under one import org.apache.wiki.event.WikiSecurityEvent; import org.apache.wiki.ui.InputValidator; import org.apache.wiki.util.ClassUtil; +import org.apache.wiki.util.Synchronizer; import java.security.Principal; import java.util.Arrays; @@ -167,16 +168,13 @@ public void initialize( final Engine engine, final Properties props ) throws Wik // Load all groups from the database into the cache final Group[] groups = m_groupDatabase.groups(); - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { for( final Group group : groups ) { // Add new group to cache; fire GROUP_ADD event m_groups.put( group.getPrincipal(), group ); fireEvent( WikiSecurityEvent.GROUP_ADD, group ); } - } finally { - lock.unlock(); - } + }); // Make the GroupManager listen for WikiEvents (WikiSecurityEvents for changed user profiles) engine.getManager( UserManager.class ).addWikiEventListener( this ); @@ -271,12 +269,9 @@ public void removeGroup( final String index ) throws WikiSecurityException { // Delete the group // TODO: need rollback procedure - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { m_groups.remove( group.getPrincipal() ); - } finally { - lock.unlock(); - } + }); m_groupDatabase.delete( group ); fireEvent( WikiSecurityEvent.GROUP_REMOVE, group ); } @@ -290,12 +285,9 @@ public void setGroup( final Session session, final Group group ) throws WikiSecu final Group oldGroup = m_groups.get( group.getPrincipal() ); if( oldGroup != null ) { fireEvent( WikiSecurityEvent.GROUP_REMOVE, oldGroup ); - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { m_groups.remove( oldGroup.getPrincipal() ); - } finally { - lock.unlock(); - } + }); } // Copy existing modifier info & timestamps @@ -307,12 +299,9 @@ public void setGroup( final Session session, final Group group ) throws WikiSecu } // Add new group to cache; announce GROUP_ADD event - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { m_groups.put( group.getPrincipal(), group ); - } finally { - lock.unlock(); - } + }); fireEvent( WikiSecurityEvent.GROUP_ADD, group ); // Save the group to back-end database; if it fails, roll back to previous state. Note that the back-end @@ -327,12 +316,9 @@ public void setGroup( final Session session, final Group group ) throws WikiSecu // Restore previous version, re-throw... fireEvent( WikiSecurityEvent.GROUP_REMOVE, group ); fireEvent( WikiSecurityEvent.GROUP_ADD, oldGroup ); - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { m_groups.put( oldGroup.getPrincipal(), oldGroup ); - } finally { - lock.unlock(); - } + }); throw new WikiSecurityException( e.getMessage() + " (rolled back to previous version).", e ); } // Re-throw security exception @@ -398,23 +384,17 @@ protected String[] extractMembers( final String memberLine ) { /** {@inheritDoc} */ @Override public void addWikiEventListener( final WikiEventListener listener ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { WikiEventManager.addWikiEventListener( this, listener ); - } finally { - lock.unlock(); - } + }); } /** {@inheritDoc} */ @Override public void removeWikiEventListener( final WikiEventListener listener ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { WikiEventManager.removeWikiEventListener( this, listener ); - } finally { - lock.unlock(); - } + }); } /** {@inheritDoc} */ diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/authorize/Group.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/authorize/Group.java index 31b67dbec1..327af4841b 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/auth/authorize/Group.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/authorize/Group.java @@ -19,10 +19,13 @@ Licensed to the Apache Software Foundation (ASF) under one package org.apache.wiki.auth.authorize; import org.apache.wiki.auth.GroupPrincipal; +import org.apache.wiki.event.WikiEventManager; +import org.apache.wiki.util.Synchronizer; import java.security.Principal; import java.util.Date; import java.util.Vector; +import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.locks.ReentrantLock; /** @@ -111,29 +114,21 @@ protected Group( final String name, final String wiki ) { * @return true if the operation was successful */ public boolean add( final Principal user ) { - lock.lock(); - try { + return Synchronizer.synchronize(lock, () -> { if( isMember( user ) ) { return false; } m_members.add( user ); return true; - } finally { - lock.unlock(); - } + }); } /** * Clears all Principals from the group list. */ public void clear() { - lock.lock(); - try { - m_members.clear(); - } finally { - lock.unlock(); - } + Synchronizer.synchronize(lock, m_members::clear); } /** @@ -179,12 +174,7 @@ public int hashCode() { * @return the creation date */ public Date getCreated() { - lock.lock(); - try { - return m_created; - } finally { - lock.unlock(); - } + return Synchronizer.synchronize(lock, () -> m_created); } /** @@ -193,12 +183,7 @@ public Date getCreated() { * @return the creator */ public final String getCreator() { - lock.lock(); - try { - return m_creator; - } finally { - lock.unlock(); - } + return Synchronizer.synchronize(lock, () -> m_creator); } /** @@ -207,12 +192,7 @@ public final String getCreator() { * @return the date and time of last modification */ public Date getLastModified() { - lock.lock(); - try { - return m_modified; - } finally { - lock.unlock(); - } + return Synchronizer.synchronize(lock, () -> m_modified); } /** @@ -221,12 +201,7 @@ public Date getLastModified() { * @return the modifier */ public final String getModifier() { - lock.lock(); - try { - return m_modifier; - } finally { - lock.unlock(); - } + return Synchronizer.synchronize(lock, () -> m_modifier); } /** @@ -282,20 +257,17 @@ public Principal[] members() { * @param user the principal to remove * @return true if the operation was successful */ - public boolean remove( Principal user ) { - lock.lock(); - try { - user = findMember( user.getName() ); - if( user == null ) + public boolean remove(Principal user) { + final AtomicReference userRef = new AtomicReference<>(user); + return Synchronizer.synchronize(lock, () -> { + Principal updatedUser = findMember(userRef.get().getName()); + userRef.set(updatedUser); + if (updatedUser == null) { return false; - - m_members.remove( user ); - + } + m_members.remove(updatedUser); return true; - } finally { - lock.unlock(); - } - + }); } /** @@ -304,12 +276,9 @@ public boolean remove( Principal user ) { * @param date the creation date */ public void setCreated( final Date date ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { m_created = date; - } finally { - lock.unlock(); - } + }); } /** @@ -317,12 +286,9 @@ public void setCreated( final Date date ) { * @param creator the creator */ public final void setCreator( final String creator ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { this.m_creator = creator; - } finally { - lock.unlock(); - } + }); } /** @@ -331,12 +297,9 @@ public final void setCreator( final String creator ) { * @param date the last-modified date */ public void setLastModified( final Date date ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { m_modified = date; - } finally { - lock.unlock(); - } + }); } /** @@ -345,12 +308,9 @@ public void setLastModified( final Date date ) { * @param modifier the modifier */ public final void setModifier( final String modifier ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { this.m_modifier = modifier; - } finally { - lock.unlock(); - } + }); } /** diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/login/CookieAuthenticationLoginModule.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/login/CookieAuthenticationLoginModule.java index 28299c2a1a..28a9634d3b 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/auth/login/CookieAuthenticationLoginModule.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/login/CookieAuthenticationLoginModule.java @@ -24,6 +24,7 @@ Licensed to the Apache Software Foundation (ASF) under one import org.apache.wiki.auth.WikiPrincipal; import org.apache.wiki.util.FileUtil; import org.apache.wiki.util.HttpUtil; +import org.apache.wiki.util.Synchronizer; import org.apache.wiki.util.TextUtil; import javax.security.auth.callback.Callback; @@ -280,8 +281,7 @@ private static Cookie getLoginCookie( final String value ) { * @param cookieDir cookie directory */ private static void scrub( final int days, final File cookieDir ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { LOG.debug( "Scrubbing cookieDir..." ); final File[] files = cookieDir.listFiles(); final long obsoleteDateLimit = System.currentTimeMillis() - ( ( long )days + 1 ) * 24 * 60 * 60 * 1000L; @@ -300,9 +300,7 @@ private static void scrub( final int days, final File cookieDir ) { } LOG.debug( "Removed {} obsolete cookie logins", deleteCount ); - } finally { - lock.unlock(); - } + }); } diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/permissions/PermissionFactory.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/permissions/PermissionFactory.java index 22d13be93f..37c6ec726f 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/auth/permissions/PermissionFactory.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/permissions/PermissionFactory.java @@ -19,8 +19,10 @@ Licensed to the Apache Software Foundation (ASF) under one package org.apache.wiki.auth.permissions; import org.apache.wiki.api.core.Page; +import org.apache.wiki.util.Synchronizer; import java.util.WeakHashMap; +import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.locks.ReentrantLock; @@ -86,45 +88,28 @@ public static PagePermission getPagePermission( final String page, final String * @param actions A list of actions. * @return A PagePermission object. */ - private static PagePermission getPagePermission( final String wiki, String page, final String actions ) - { - PagePermission perm; - // - // Since this is pretty speed-critical, we try to avoid the StringBuffer creation - // overhead by XORring the hashcodes. However, if page name length > 32 characters, - // this might result in two same hashCodes. - // FIXME: Make this work for page-name lengths > 32 characters (use the alt implementation - // if page.length() > 32?) - // Alternative implementation below, but it does create an extra StringBuffer. - //String key = wiki+":"+page+":"+actions; - + private static PagePermission getPagePermission(final String wiki, String page, final String actions) { final Integer key = wiki.hashCode() ^ page.hashCode() ^ actions.hashCode(); - - // - // It's fine if two threads update the cache, since the objects mean the same - // thing anyway. And this avoids nasty blocking effects. - // - lock.lock(); - try { - perm = c_cache.get( key ); - } finally { - lock.unlock(); - } - - if( perm == null ) - { - if( !wiki.isEmpty() ) page = wiki+":"+page; - perm = new PagePermission( page, actions ); + AtomicReference permRef = new AtomicReference<>(); + + Synchronizer.synchronize(lock, () -> { + PagePermission perm = c_cache.get(key); + permRef.set(perm); + }); + + if (permRef.get() == null) { + if (!wiki.isEmpty()) page = wiki + ":" + page; + PagePermission newPerm = new PagePermission(page, actions); - lock.lock(); - try { - c_cache.put( key, perm ); - } finally { - lock.unlock(); - } + Synchronizer.synchronize(lock, () -> { + c_cache.put(key, newPerm); + }); + + return newPerm; } - - return perm; + + return permRef.get(); } + } diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/user/XMLUserDatabase.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/user/XMLUserDatabase.java index edfe1b2d45..b88b94310c 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/auth/user/XMLUserDatabase.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/user/XMLUserDatabase.java @@ -25,6 +25,7 @@ Licensed to the Apache Software Foundation (ASF) under one import org.apache.wiki.auth.WikiPrincipal; import org.apache.wiki.auth.WikiSecurityException; import org.apache.wiki.util.Serializer; +import org.apache.wiki.util.Synchronizer; import org.apache.wiki.util.TextUtil; import org.w3c.dom.Document; import org.w3c.dom.Element; @@ -52,6 +53,8 @@ Licensed to the Apache Software Foundation (ASF) under one import java.util.Properties; import java.util.SortedSet; import java.util.TreeSet; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.locks.ReentrantLock; import java.util.stream.Collectors; import java.util.stream.IntStream; @@ -107,31 +110,44 @@ public XMLUserDatabase() { } /** {@inheritDoc} */ - @Override - public void deleteByLoginName( final String loginName ) throws WikiSecurityException { - lock.lock(); - try { - if( c_dom == null ) { - throw new WikiSecurityException( "FATAL: database does not exist" ); - } + public void deleteByLoginName(final String loginName) throws WikiSecurityException { + final AtomicBoolean userDeleted = new AtomicBoolean(false); + final AtomicReference exceptionRef = new AtomicReference<>(); - final NodeList users = c_dom.getDocumentElement().getElementsByTagName( USER_TAG ); - for( int i = 0; i < users.getLength(); i++ ) { - final Element user = ( Element )users.item( i ); - if( user.getAttribute( LOGIN_NAME ).equals( loginName ) ) { - c_dom.getDocumentElement().removeChild( user ); + Synchronizer.synchronize(lock, () -> { + try { + if (c_dom == null) { + throw new WikiSecurityException("FATAL: database does not exist"); + } - // Commit to disk - saveDOM(); - return; + final NodeList users = c_dom.getDocumentElement().getElementsByTagName(USER_TAG); + for (int i = 0; i < users.getLength(); i++) { + final Element user = (Element) users.item(i); + if (user.getAttribute(LOGIN_NAME).equals(loginName)) { + c_dom.getDocumentElement().removeChild(user); + + // Commit to disk + saveDOM(); + userDeleted.set(true); + return; + } } + } catch (WikiSecurityException e) { + exceptionRef.set(e); } - throw new NoSuchPrincipalException( "Not in database: " + loginName ); - } finally { - lock.unlock(); + }); + + if (exceptionRef.get() != null) { + throw exceptionRef.get(); + } + + if (!userDeleted.get()) { + throw new NoSuchPrincipalException("Not in database: " + loginName); } } + + /** {@inheritDoc} */ @Override public UserProfile findByEmail( final String index ) throws NoSuchPrincipalException { @@ -343,55 +359,51 @@ private void checkForRefresh() { */ @Override public void rename( final String loginName, final String newName) throws DuplicateUserException, WikiSecurityException { - lock.lock(); - try { - if( c_dom == null ) { - LOG.fatal( "Could not rename profile '" + loginName + "'; database does not exist" ); - throw new IllegalStateException( "FATAL: database does not exist" ); + Synchronizer.synchronize(lock, () -> { + if (c_dom == null) { + LOG.fatal("Could not rename profile '" + loginName + "'; database does not exist"); + throw new IllegalStateException("FATAL: database does not exist"); } checkForRefresh(); // Get the existing user; if not found, throws NoSuchPrincipalException - final UserProfile profile = findByLoginName( loginName ); + final UserProfile profile = findByLoginName(loginName); // Get user with the proposed name; if found, it's a collision try { - final UserProfile otherProfile = findByLoginName( newName ); - if( otherProfile != null ) { - throw new DuplicateUserException( "security.error.cannot.rename", newName ); + final UserProfile otherProfile = findByLoginName(newName); + if (otherProfile != null) { + throw new DuplicateUserException("security.error.cannot.rename", newName); } - } catch( final NoSuchPrincipalException e ) { + } catch (final NoSuchPrincipalException e) { // Good! That means it's safe to save using the new name } // Find the user with the old login id attribute, and change it - final NodeList users = c_dom.getElementsByTagName( USER_TAG ); - for( int i = 0; i < users.getLength(); i++ ) { - final Element user = ( Element )users.item( i ); - if( user.getAttribute( LOGIN_NAME ).equals( loginName ) ) { - final DateFormat c_format = new SimpleDateFormat( DATE_FORMAT ); - final Date modDate = new Date( System.currentTimeMillis() ); - setAttribute( user, LOGIN_NAME, newName ); - setAttribute( user, LAST_MODIFIED, c_format.format( modDate ) ); - profile.setLoginName( newName ); - profile.setLastModified( modDate ); + final NodeList users = c_dom.getElementsByTagName(USER_TAG); + for (int i = 0; i < users.getLength(); i++) { + final Element user = (Element) users.item(i); + if (user.getAttribute(LOGIN_NAME).equals(loginName)) { + final DateFormat c_format = new SimpleDateFormat(DATE_FORMAT); + final Date modDate = new Date(System.currentTimeMillis()); + setAttribute(user, LOGIN_NAME, newName); + setAttribute(user, LAST_MODIFIED, c_format.format(modDate)); + profile.setLoginName(newName); + profile.setLastModified(modDate); break; } } // Commit to disk saveDOM(); - } finally { - lock.unlock(); - } + }); } /** {@inheritDoc} */ @Override public void save( final UserProfile profile ) throws WikiSecurityException { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { if ( c_dom == null ) { LOG.fatal( "Could not save profile " + profile + " database does not exist" ); throw new IllegalStateException( "FATAL: database does not exist" ); @@ -462,9 +474,7 @@ public void save( final UserProfile profile ) throws WikiSecurityException { // Commit to disk saveDOM(); - } finally { - lock.unlock(); - } + }); } /** diff --git a/jspwiki-main/src/main/java/org/apache/wiki/diff/ContextualDiffProvider.java b/jspwiki-main/src/main/java/org/apache/wiki/diff/ContextualDiffProvider.java index bf4c71aa5c..0d70680e5e 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/diff/ContextualDiffProvider.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/diff/ContextualDiffProvider.java @@ -24,6 +24,7 @@ Licensed to the Apache Software Foundation (ASF) under one import org.apache.wiki.api.core.Context; import org.apache.wiki.api.core.Engine; import org.apache.wiki.api.exceptions.NoRequiredPropertyException; +import org.apache.wiki.util.Synchronizer; import org.apache.wiki.util.TextUtil; import org.suigeneris.jrcs.diff.Diff; import org.suigeneris.jrcs.diff.DifferentiationFailedException; @@ -151,8 +152,7 @@ public void initialize( final Engine engine, final Properties properties) throws */ @Override public String makeDiffHtml( final Context ctx, final String wikiOld, final String wikiNew ) { - lock.lock(); - try { + return Synchronizer.synchronize(lock, () -> { // // Sequencing handles lineterminator to
and every-other consequtive space to a   // @@ -181,9 +181,7 @@ public String makeDiffHtml( final Context ctx, final String wikiOld, final Strin cm.shutdown(); sb.append( DIFF_END ); return sb.toString(); - } finally { - lock.unlock(); - } + }); } /** diff --git a/jspwiki-main/src/main/java/org/apache/wiki/filters/PageEventFilter.java b/jspwiki-main/src/main/java/org/apache/wiki/filters/PageEventFilter.java index b58c3f249b..bdbed514e6 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/filters/PageEventFilter.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/filters/PageEventFilter.java @@ -26,6 +26,7 @@ Licensed to the Apache Software Foundation (ASF) under one import org.apache.wiki.event.WikiEventListener; import org.apache.wiki.event.WikiEventManager; import org.apache.wiki.event.WikiPageEvent; +import org.apache.wiki.util.Synchronizer; import java.util.Properties; import java.util.concurrent.locks.ReentrantLock; @@ -128,12 +129,9 @@ public void postSave( final Context wikiContext, final String content ) { * @param listener the event listener */ public final void addWikiEventListener( final WikiEventListener listener ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { WikiEventManager.addWikiEventListener( this, listener ); - } finally { - lock.unlock(); - } + }); } /** @@ -142,12 +140,9 @@ public final void addWikiEventListener( final WikiEventListener listener ) { * @param listener the event listener */ public final void removeWikiEventListener( final WikiEventListener listener ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { WikiEventManager.removeWikiEventListener( this, listener ); - } finally { - lock.unlock(); - } + }); } /** diff --git a/jspwiki-main/src/main/java/org/apache/wiki/filters/SpamFilter.java b/jspwiki-main/src/main/java/org/apache/wiki/filters/SpamFilter.java index 981a291b6a..4c51af8a44 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/filters/SpamFilter.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/filters/SpamFilter.java @@ -46,6 +46,7 @@ Licensed to the Apache Software Foundation (ASF) under one import org.apache.wiki.ui.EditorManager; import org.apache.wiki.util.FileUtil; import org.apache.wiki.util.HttpUtil; +import org.apache.wiki.util.Synchronizer; import org.apache.wiki.util.TextUtil; import org.suigeneris.jrcs.diff.Diff; import org.suigeneris.jrcs.diff.DifferentiationFailedException; @@ -452,8 +453,7 @@ private Collection< Pattern > parseBlacklist( final String list ) { */ private void checkSinglePageChange(final Context context, final Change change ) throws RedirectException { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { final HttpServletRequest req = context.getHttpRequest(); if( req != null ) { @@ -534,9 +534,7 @@ private void checkSinglePageChange(final Context context, final Change change ) m_lastModifications.add( new Host( addr, change ) ); } - } finally { - lock.unlock(); - } + }); } @@ -651,8 +649,7 @@ private void checkUTF8( final Context context, final Change change ) throws Redi /** Goes through the ban list and cleans away any host which has expired from it. */ private void cleanBanList() { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { final long now = System.currentTimeMillis(); for( final Iterator< Host > i = m_temporaryBanList.iterator(); i.hasNext(); ) { final Host host = i.next(); @@ -662,9 +659,7 @@ private void cleanBanList() { i.remove(); } } - } finally { - lock.unlock(); - } + }); } /** diff --git a/jspwiki-main/src/main/java/org/apache/wiki/plugin/BugReportHandler.java b/jspwiki-main/src/main/java/org/apache/wiki/plugin/BugReportHandler.java index ad59bef784..090917ae38 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/plugin/BugReportHandler.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/plugin/BugReportHandler.java @@ -31,6 +31,7 @@ Licensed to the Apache Software Foundation (ASF) under one import org.apache.wiki.pages.PageManager; import org.apache.wiki.parser.MarkupParser; import org.apache.wiki.preferences.Preferences; +import org.apache.wiki.util.Synchronizer; import org.apache.wiki.util.TextUtil; import java.io.PrintWriter; @@ -182,8 +183,7 @@ public String execute( final Context context, final Map< String, String > params * and tries again. */ private String findNextPage( final Context context, final String title, final String baseName ) { - lock.lock(); - try { + return Synchronizer.synchronize(lock, () -> { final String basicPageName = ( ( baseName != null ) ? baseName : "Bug" ) + MarkupParser.cleanLink( title ); final Engine engine = context.getEngine(); @@ -192,11 +192,8 @@ private String findNextPage( final Context context, final String title, final St while( engine.getManager( PageManager.class ).wikiPageExists( pageName ) ) { pageName = basicPageName + lastbug++; } - return pageName; - } finally { - lock.unlock(); - } + }); } /** diff --git a/jspwiki-main/src/main/java/org/apache/wiki/plugin/PageViewPlugin.java b/jspwiki-main/src/main/java/org/apache/wiki/plugin/PageViewPlugin.java index 53b8859ca1..1e8de34814 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/plugin/PageViewPlugin.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/plugin/PageViewPlugin.java @@ -43,6 +43,7 @@ Licensed to the Apache Software Foundation (ASF) under one import org.apache.wiki.event.WikiPageRenameEvent; import org.apache.wiki.references.ReferenceManager; import org.apache.wiki.render.RenderingManager; +import org.apache.wiki.util.Synchronizer; import org.apache.wiki.util.TextUtil; import java.io.File; @@ -152,15 +153,12 @@ public PageViewPlugin() { @Override public void initialize( final Engine engine ) { LOG.info( "initializing PageViewPlugin" ); - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { if( c_singleton == null ) { c_singleton = new PageViewManager(); } c_singleton.initialize( engine ); - } finally { - lock.unlock(); - } + }); } /** @@ -221,8 +219,7 @@ public final class PageViewManager implements WikiEventListener { * @param engine The wiki engine. */ public void initialize( final Engine engine ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { LOG.info( "initializing PageView Manager" ); m_workDir = engine.getWorkDir(); engine.addWikiEventListener( this ); @@ -241,17 +238,14 @@ public void initialize( final Engine engine ) { } m_initialized = true; - } finally { - lock.unlock(); - } + }); } /** * Handle the shutdown event via the page counter thread. */ private void handleShutdown() { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { LOG.info( "handleShutdown: The counter store thread was shut down." ); cleanup(); @@ -271,9 +265,7 @@ private void handleShutdown() { m_initialized = false; m_pageCountSaveThread = null; - } finally { - lock.unlock(); - } + }); } /** @@ -541,8 +533,7 @@ int getCount( final Object key ) private void loadCounters() { if( m_counters != null && m_storage != null ) { LOG.info( "Loading counters." ); - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { try( final InputStream fis = Files.newInputStream( new File( m_workDir, COUNTER_PAGE ).toPath() ) ) { m_storage.load( fis ); } catch( final IOException ioe ) { @@ -555,9 +546,7 @@ private void loadCounters() { } LOG.info( "Loaded " + m_counters.size() + " counter values." ); - } finally { - lock.unlock(); - } + }); } } @@ -567,8 +556,7 @@ private void loadCounters() { void storeCounters() { if( m_counters != null && m_storage != null && m_dirty ) { LOG.info( "Storing " + m_counters.size() + " counter values." ); - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { // Write out the collection of counters try( final OutputStream fos = Files.newOutputStream( new File( m_workDir, COUNTER_PAGE ).toPath() ) ) { m_storage.store( fos, "\n# The number of times each page has been viewed.\n# Do not modify.\n" ); @@ -578,9 +566,7 @@ void storeCounters() { } catch( final IOException ioe ) { LOG.error( "Couldn't store counters values: " + ioe.getMessage() ); } - } finally { - lock.unlock(); - } + }); } } @@ -592,13 +578,7 @@ void storeCounters() { */ private boolean isRunning( final Thread thrd ) { - lock.lock(); - try { - return m_initialized && thrd == m_pageCountSaveThread; - } finally { - lock.unlock(); - } - + return Synchronizer.synchronize(lock, () -> m_initialized && thrd == m_pageCountSaveThread); } } diff --git a/jspwiki-main/src/main/java/org/apache/wiki/providers/CachingAttachmentProvider.java b/jspwiki-main/src/main/java/org/apache/wiki/providers/CachingAttachmentProvider.java index 8d5bcf415d..9b94ce2495 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/providers/CachingAttachmentProvider.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/providers/CachingAttachmentProvider.java @@ -33,6 +33,7 @@ Licensed to the Apache Software Foundation (ASF) under one import org.apache.wiki.cache.CacheInfo; import org.apache.wiki.cache.CachingManager; import org.apache.wiki.util.ClassUtil; +import org.apache.wiki.util.Synchronizer; import org.apache.wiki.util.TextUtil; import java.io.IOException; @@ -156,8 +157,7 @@ public List< Attachment > listAllChanged( final Date timestamp ) throws Provider all = provider.listAllChanged( timestamp ); // Make sure that all attachments are in the cache. - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { for( final Attachment att : all ) { cachingManager.put( CachingManager.CACHE_ATTACHMENTS, att.getName(), att ); } @@ -165,9 +165,7 @@ public List< Attachment > listAllChanged( final Date timestamp ) throws Provider allRequested = true; attachments.set( all.size() ); } - } finally { - lock.unlock(); - } + }); } else { final List< String > keys = cachingManager.keys( CachingManager.CACHE_ATTACHMENTS ); all = new ArrayList<>(); @@ -259,8 +257,7 @@ public void deleteAttachment( final Attachment att ) throws ProviderException { */ @Override public String getProviderInfo() { - lock.lock(); - try { + return Synchronizer.synchronize(lock, () -> { final CacheInfo attCacheInfo = cachingManager.info( CachingManager.CACHE_ATTACHMENTS ); final CacheInfo attColCacheInfo = cachingManager.info( CachingManager.CACHE_ATTACHMENTS_COLLECTION ); return "Real provider: " + provider.getClass().getName() + @@ -268,9 +265,7 @@ public String getProviderInfo() { ". Attachment cache misses: " + attCacheInfo.getMisses() + ". Attachment collection cache hits: " + attColCacheInfo.getHits() + ". Attachment collection cache misses: " + attColCacheInfo.getMisses(); - } finally { - lock.unlock(); - } + }); } diff --git a/jspwiki-main/src/main/java/org/apache/wiki/providers/CachingProvider.java b/jspwiki-main/src/main/java/org/apache/wiki/providers/CachingProvider.java index 745e58b734..bc66c43aa8 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/providers/CachingProvider.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/providers/CachingProvider.java @@ -35,6 +35,7 @@ Licensed to the Apache Software Foundation (ASF) under one import org.apache.wiki.parser.MarkupParser; import org.apache.wiki.render.RenderingManager; import org.apache.wiki.util.ClassUtil; +import org.apache.wiki.util.Synchronizer; import org.apache.wiki.util.TextUtil; import java.io.IOException; @@ -235,8 +236,7 @@ private String getTextFromCache( final String pageName ) throws ProviderExceptio */ @Override public void putPageText( final Page page, final String text ) throws ProviderException { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { provider.putPageText( page, text ); page.setLastModified( new Date() ); @@ -246,9 +246,7 @@ public void putPageText( final Page page, final String text ) throws ProviderExc cachingManager.remove( CachingManager.CACHE_PAGES_HISTORY, page.getName() ); getPageInfoFromCache( page.getName() ); - } finally { - lock.unlock(); - } + }); pages.incrementAndGet(); } @@ -261,15 +259,12 @@ public Collection< Page > getAllPages() throws ProviderException { if ( !allRequested ) { all = provider.getAllPages(); // Make sure that all pages are in the cache. - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { for( final Page p : all ) { cachingManager.put( CachingManager.CACHE_PAGES, p.getName(), p ); } allRequested = true; - } finally { - lock.unlock(); - } + }); pages.set( all.size() ); } else { final List< String > keys = cachingManager.keys( CachingManager.CACHE_PAGES ); @@ -371,8 +366,7 @@ public List< Page > getVersionHistory( final String pageName) throws ProviderExc */ @Override public String getProviderInfo() { - lock.lock(); - try { + return Synchronizer.synchronize(lock, () -> { final CacheInfo pageCacheInfo = cachingManager.info( CachingManager.CACHE_PAGES ); final CacheInfo pageHistoryCacheInfo = cachingManager.info( CachingManager.CACHE_PAGES_HISTORY ); return "Real provider: " + provider.getClass().getName()+ @@ -380,9 +374,7 @@ public String getProviderInfo() { ". Page cache misses: " + pageCacheInfo.getMisses() + ". History cache hits: " + pageHistoryCacheInfo.getHits() + ". History cache misses: " + pageHistoryCacheInfo.getMisses(); - } finally { - lock.unlock(); - } + }); } /** @@ -391,8 +383,7 @@ public String getProviderInfo() { @Override public void deleteVersion( final String pageName, final int version ) throws ProviderException { // Luckily, this is such a rare operation it is okay to synchronize against the whole thing. - lock.lock(); - try { + Synchronizer.synchronize(lock, (Synchronizer.ThrowingRunnable) () -> { final Page cached = getPageInfoFromCache( pageName ); final int latestcached = ( cached != null ) ? cached.getVersion() : Integer.MIN_VALUE; @@ -404,9 +395,7 @@ public void deleteVersion( final String pageName, final int version ) throws Pro provider.deleteVersion( pageName, version ); cachingManager.remove( CachingManager.CACHE_PAGES_HISTORY, pageName ); - } finally { - lock.unlock(); - } + }); if( version == PageProvider.LATEST_VERSION ) { pages.decrementAndGet(); } @@ -418,15 +407,12 @@ public void deleteVersion( final String pageName, final int version ) throws Pro @Override public void deletePage( final String pageName ) throws ProviderException { // See note in deleteVersion(). - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { cachingManager.put( CachingManager.CACHE_PAGES, pageName, null ); cachingManager.put( CachingManager.CACHE_PAGES_TEXT, pageName, null ); cachingManager.put( CachingManager.CACHE_PAGES_HISTORY, pageName, null ); provider.deletePage( pageName ); - } finally { - lock.unlock(); - } + }); pages.decrementAndGet(); } @@ -436,8 +422,7 @@ public void deletePage( final String pageName ) throws ProviderException { @Override public void movePage( final String from, final String to ) throws ProviderException { provider.movePage( from, to ); - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { // Clear any cached version of the old page and new page cachingManager.remove( CachingManager.CACHE_PAGES, from ); cachingManager.remove( CachingManager.CACHE_PAGES_TEXT, from ); @@ -446,9 +431,7 @@ public void movePage( final String from, final String to ) throws ProviderExcept cachingManager.remove( CachingManager.CACHE_PAGES, to ); cachingManager.remove( CachingManager.CACHE_PAGES_TEXT, to ); cachingManager.remove( CachingManager.CACHE_PAGES_HISTORY, to ); - } finally { - lock.unlock(); - } + }); } /** diff --git a/jspwiki-main/src/main/java/org/apache/wiki/providers/VersioningFileProvider.java b/jspwiki-main/src/main/java/org/apache/wiki/providers/VersioningFileProvider.java index d6908853ca..9f814df565 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/providers/VersioningFileProvider.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/providers/VersioningFileProvider.java @@ -29,6 +29,7 @@ Licensed to the Apache Software Foundation (ASF) under one import org.apache.wiki.api.providers.WikiProvider; import org.apache.wiki.api.spi.Wiki; import org.apache.wiki.util.FileUtil; +import org.apache.wiki.util.Synchronizer; import java.io.BufferedInputStream; import java.io.BufferedOutputStream; @@ -42,6 +43,8 @@ Licensed to the Apache Software Foundation (ASF) under one import java.util.Date; import java.util.List; import java.util.Properties; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.locks.ReentrantLock; /** @@ -283,26 +286,29 @@ private int realVersion( final String page, final int requestedVersion ) throws * {@inheritDoc} */ @Override - public String getPageText( final String page, int version ) throws ProviderException { - lock.lock(); - try { - final File dir = findOldPageDir( page ); + public String getPageText(final String page, int version) throws ProviderException { + final AtomicReference result = new AtomicReference<>(); + final int finalVersion = version; // Make it effectively final for use in lambda - version = realVersion( page, version ); - if( version == -1 ) { + Synchronizer.synchronize(lock, () -> { + final File dir = findOldPageDir(page); + int realVersion = realVersion(page, finalVersion); + + if (realVersion == -1) { // We can let the FileSystemProvider take care of these requests. - return super.getPageText( page, PageProvider.LATEST_VERSION ); + result.set(super.getPageText(page, PageProvider.LATEST_VERSION)); + return; } - final File pageFile = new File( dir, ""+version+FILE_EXT ); - if( !pageFile.exists() ) { - throw new NoSuchVersionException("Version "+version+"does not exist."); + final File pageFile = new File(dir, "" + realVersion + FILE_EXT); + if (!pageFile.exists()) { + throw new NoSuchVersionException("Version " + realVersion + " does not exist."); } - return readFile( pageFile ); - } finally { - lock.unlock(); - } + result.set(readFile(pageFile)); + }); + + return result.get(); } @@ -346,8 +352,7 @@ private String readFile( final File pagedata ) throws ProviderException { */ @Override public void putPageText( final Page page, final String text ) throws ProviderException { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { // This is a bit complicated. We'll first need to copy the old file to be the newest file. final int latest = findLatestVersion( page.getName() ); final File pageDir = findOldPageDir( page.getName() ); @@ -417,9 +422,7 @@ public void putPageText( final Page page, final String text ) throws ProviderExc LOG.error( "Saving failed", e ); throw new ProviderException("Could not save page text: "+e.getMessage()); } - } finally { - lock.unlock(); - } + }); } diff --git a/jspwiki-main/src/main/java/org/apache/wiki/references/DefaultReferenceManager.java b/jspwiki-main/src/main/java/org/apache/wiki/references/DefaultReferenceManager.java index 7607b19e08..c753f5b94a 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/references/DefaultReferenceManager.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/references/DefaultReferenceManager.java @@ -38,6 +38,7 @@ Licensed to the Apache Software Foundation (ASF) under one import org.apache.wiki.event.WikiPageEvent; import org.apache.wiki.pages.PageManager; import org.apache.wiki.render.RenderingManager; +import org.apache.wiki.util.Synchronizer; import org.apache.wiki.util.TextUtil; import java.io.*; @@ -47,6 +48,7 @@ Licensed to the Apache Software Foundation (ASF) under one import java.security.NoSuchAlgorithmException; import java.util.*; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.locks.ReentrantLock; import java.util.stream.Collectors; @@ -246,63 +248,61 @@ public void initialize( final Collection< Page > pages ) throws ProviderExceptio */ @SuppressWarnings("unchecked") private long unserializeFromDisk() throws IOException, ClassNotFoundException { - lock.lock(); - try { - final long saved; - - final File f = new File( m_engine.getWorkDir(), SERIALIZATION_FILE ); - try( final ObjectInputStream in = new ObjectInputStream( new BufferedInputStream( Files.newInputStream( f.toPath() ) ) ) ) { - final StopWatch sw = new StopWatch(); - sw.start(); + final AtomicLong saved = new AtomicLong(0); + Synchronizer.synchronize(lock, () -> { + try { + final File f = new File(m_engine.getWorkDir(), SERIALIZATION_FILE); + try (final ObjectInputStream in = new ObjectInputStream(new BufferedInputStream(Files.newInputStream(f.toPath())))) { + final StopWatch sw = new StopWatch(); + sw.start(); - final long ver = in.readLong(); + final long ver = in.readLong(); - if( ver != serialVersionUID ) { - throw new IOException("File format has changed; I need to recalculate references."); - } + if (ver != serialVersionUID) { + throw new IOException("File format has changed; I need to recalculate references."); + } - saved = in.readLong(); - m_refersTo = ( Map< String, Collection< String > > ) in.readObject(); - m_referredBy = ( Map< String, Set< String > > ) in.readObject(); + saved.set(in.readLong()); + m_refersTo = (Map>) in.readObject(); + m_referredBy = (Map>) in.readObject(); - m_unmutableReferredBy = Collections.unmodifiableMap( m_referredBy ); - m_unmutableRefersTo = Collections.unmodifiableMap( m_refersTo ); + m_unmutableReferredBy = Collections.unmodifiableMap(m_referredBy); + m_unmutableRefersTo = Collections.unmodifiableMap(m_refersTo); - sw.stop(); - LOG.debug( "Read serialized data successfully in {}", sw ); + sw.stop(); + LOG.debug("Read serialized data successfully in {}", sw); + } + } catch (IOException | ClassNotFoundException e) { + throw new RuntimeException(e); } + }); - return saved; - } finally { - lock.unlock(); - } + return saved.get(); } + /** * Serializes hashmaps to disk. The format is private, don't touch it. */ private void serializeToDisk() { - lock.lock(); - try { - final File f = new File( m_engine.getWorkDir(), SERIALIZATION_FILE ); - try( final ObjectOutputStream out = new ObjectOutputStream( new BufferedOutputStream( Files.newOutputStream( f.toPath() ) ) ) ) { + Synchronizer.synchronize(lock, () -> { + final File f = new File(m_engine.getWorkDir(), SERIALIZATION_FILE); + try (final ObjectOutputStream out = new ObjectOutputStream(new BufferedOutputStream(Files.newOutputStream(f.toPath())))) { final StopWatch sw = new StopWatch(); sw.start(); - out.writeLong( serialVersionUID ); - out.writeLong( System.currentTimeMillis() ); // Timestamp - out.writeObject( m_refersTo ); - out.writeObject( m_referredBy ); + out.writeLong(serialVersionUID); + out.writeLong(System.currentTimeMillis()); // Timestamp + out.writeObject(m_refersTo); + out.writeObject(m_referredBy); sw.stop(); - LOG.debug( "serialization done - took {}", sw ); - } catch( final IOException ioe ) { - LOG.error( "Unable to serialize!", ioe ); + LOG.debug("serialization done - took {}", sw); + } catch (final IOException ioe) { + LOG.error("Unable to serialize!", ioe); } - } finally { - lock.unlock(); - } + }); } private String getHashFileName( final String pageName ) { @@ -379,8 +379,7 @@ private long unserializeAttrsFromDisk( final Page p ) throws IOException, ClassN * Serializes hashmaps to disk. The format is private, don't touch it. */ private void serializeAttrsToDisk( final Page p ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { final StopWatch sw = new StopWatch(); sw.start(); @@ -424,9 +423,7 @@ private void serializeAttrsToDisk( final Page p ) { LOG.debug( "serialization for {} done - took {}", p.getName(), sw ); } } - } finally { - lock.unlock(); - } + }); } /** diff --git a/jspwiki-main/src/main/java/org/apache/wiki/rss/DefaultRSSGenerator.java b/jspwiki-main/src/main/java/org/apache/wiki/rss/DefaultRSSGenerator.java index 2e3122137d..34834ea69e 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/rss/DefaultRSSGenerator.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/rss/DefaultRSSGenerator.java @@ -34,6 +34,7 @@ Licensed to the Apache Software Foundation (ASF) under one import org.apache.wiki.pages.PageManager; import org.apache.wiki.pages.PageTimeComparator; import org.apache.wiki.render.RenderingManager; +import org.apache.wiki.util.Synchronizer; import org.apache.wiki.util.TextUtil; import org.apache.wiki.variables.VariableManager; @@ -213,23 +214,15 @@ public String generateFeed( final Context wikiContext, final List< Page > change /** {@inheritDoc} */ @Override public boolean isEnabled() { - lock.lock(); - try { - return m_enabled; - } finally { - lock.unlock(); - } + return Synchronizer.synchronize(lock, () -> m_enabled); } /** {@inheritDoc} */ @Override public void setEnabled( final boolean enabled ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { m_enabled = enabled; - } finally { - lock.unlock(); - } + }); } /** {@inheritDoc} */ diff --git a/jspwiki-main/src/main/java/org/apache/wiki/search/LuceneSearchProvider.java b/jspwiki-main/src/main/java/org/apache/wiki/search/LuceneSearchProvider.java index ec1848d1ea..e9b126e028 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/search/LuceneSearchProvider.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/search/LuceneSearchProvider.java @@ -67,6 +67,7 @@ Licensed to the Apache Software Foundation (ASF) under one import org.apache.wiki.pages.PageManager; import org.apache.wiki.util.ClassUtil; import org.apache.wiki.util.FileUtil; +import org.apache.wiki.util.Synchronizer; import org.apache.wiki.util.TextUtil; import java.io.File; @@ -319,8 +320,7 @@ protected String getAttachmentContent( final Attachment att ) { * @param text The page text to index. */ protected void updateLuceneIndex( final Page page, final String text ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { LOG.debug( "Updating Lucene index for page '{}'...", page.getName() ); pageRemoved( page ); @@ -337,9 +337,7 @@ protected void updateLuceneIndex( final Page page, final String text ) { } LOG.debug( "Done updating Lucene index for page '{}'.", page.getName() ); - } finally { - lock.unlock(); - } + }); } private Analyzer getLuceneAnalyzer() throws ProviderException { @@ -409,12 +407,9 @@ protected Document luceneIndexPage( final Page page, final String text, final In field = new Field( LUCENE_PAGE_KEYWORDS, page.getAttribute( "keywords" ).toString(), TextField.TYPE_STORED ); doc.add( field ); } - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { writer.addDocument( doc ); - } finally { - lock.unlock(); - } + }); return doc; } @@ -424,8 +419,7 @@ protected Document luceneIndexPage( final Page page, final String text, final In */ @Override public void pageRemoved( final Page page ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { try( final Directory luceneDir = new NIOFSDirectory( new File( m_luceneDirectory ).toPath() ); final IndexWriter writer = getIndexWriter( luceneDir ) ) { final Query query = new TermQuery( new Term( LUCENE_ID, page.getName() ) ); @@ -433,9 +427,7 @@ public void pageRemoved( final Page page ) { } catch( final Exception e ) { LOG.error( "Unable to remove page '{}' from Lucene index", page.getName(), e ); } - } finally { - lock.unlock(); - } + }); } IndexWriter getIndexWriter( final Directory luceneDir ) throws IOException, ProviderException { @@ -605,17 +597,14 @@ public void startupTask() throws Exception { @Override public void backgroundTask() { m_watchdog.enterState( "Emptying index queue", 60 ); - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { while( m_provider.m_updates.size() > 0 ) { final Object[] pair = m_provider.m_updates.remove( 0 ); final Page page = ( Page )pair[ 0 ]; final String text = ( String )pair[ 1 ]; m_provider.updateLuceneIndex( page, text ); } - } finally { - lock.unlock(); - } + }); m_watchdog.exitState(); diff --git a/jspwiki-main/src/main/java/org/apache/wiki/ui/admin/beans/SearchManagerBean.java b/jspwiki-main/src/main/java/org/apache/wiki/ui/admin/beans/SearchManagerBean.java index d85ec435c0..aa0b803f68 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/ui/admin/beans/SearchManagerBean.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/ui/admin/beans/SearchManagerBean.java @@ -27,6 +27,7 @@ Licensed to the Apache Software Foundation (ASF) under one import org.apache.wiki.ui.admin.SimpleAdminBean; import org.apache.wiki.ui.progress.ProgressItem; import org.apache.wiki.ui.progress.ProgressManager; +import org.apache.wiki.util.Synchronizer; import javax.management.NotCompliantMBeanException; import java.util.Collection; @@ -87,8 +88,7 @@ public String getTitle() * This method prevents itself from being called twice. */ public void reload() { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { if( m_updater == null ) { m_updater = new WikiBackgroundThread( m_engine, 0 ) { @@ -132,9 +132,7 @@ public int getProgress() { m_updater.start(); } - } finally { - lock.unlock(); - } + }); } @Override diff --git a/jspwiki-main/src/main/java/org/apache/wiki/workflow/AbstractStep.java b/jspwiki-main/src/main/java/org/apache/wiki/workflow/AbstractStep.java index 20be5cb527..a617760035 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/workflow/AbstractStep.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/workflow/AbstractStep.java @@ -20,6 +20,7 @@ Licensed to the Apache Software Foundation (ASF) under one import org.apache.wiki.api.core.Context; import org.apache.wiki.api.exceptions.WikiException; +import org.apache.wiki.util.Synchronizer; import java.io.Serializable; import java.security.Principal; @@ -158,12 +159,7 @@ public final String getMessageKey() { */ @Override public final Outcome getOutcome() { - lock.lock(); - try { - return m_outcome; - } finally { - lock.unlock(); - } + return Synchronizer.synchronize(lock, () -> m_outcome); } /** @@ -195,8 +191,7 @@ public final boolean isStarted() { */ @Override public final void setOutcome(final Outcome outcome ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { // Is this an allowed Outcome? if( !m_successors.containsKey( outcome ) ) { if( !Outcome.STEP_CONTINUE.equals( outcome ) && !Outcome.STEP_ABORT.equals( outcome ) ) { @@ -213,9 +208,7 @@ public final void setOutcome(final Outcome outcome ) { m_end = new Date( System.currentTimeMillis() ); } m_outcome = outcome; - } finally { - lock.unlock(); - } + }); } @@ -224,16 +217,13 @@ public final void setOutcome(final Outcome outcome ) { */ @Override public final void start() throws WikiException { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { if( m_started ) { throw new IllegalStateException( "Step already started." ); } m_started = true; m_start = new Date( System.currentTimeMillis() ); - } finally { - lock.unlock(); - } + }); } /** @@ -254,13 +244,10 @@ public final Step getSuccessor(final Outcome outcome ) { */ @Override public final void setWorkflow(final int workflowId, final Map< String, Serializable > workflowContext ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { this.workflowId = workflowId; this.workflowContext = workflowContext; - } finally { - lock.unlock(); - } + }); } public int getWorkflowId() { @@ -277,12 +264,9 @@ public Map< String, Serializable > getWorkflowContext() { * @param message the error message */ protected final void addError( final String message ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { m_errors.add( message ); - } finally { - lock.unlock(); - } + }); } } diff --git a/jspwiki-main/src/main/java/org/apache/wiki/workflow/Decision.java b/jspwiki-main/src/main/java/org/apache/wiki/workflow/Decision.java index c7f6393002..4d346e1ce8 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/workflow/Decision.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/workflow/Decision.java @@ -22,6 +22,7 @@ Licensed to the Apache Software Foundation (ASF) under one import org.apache.wiki.api.exceptions.WikiException; import org.apache.wiki.event.WikiEventEmitter; import org.apache.wiki.event.WorkflowEvent; +import org.apache.wiki.util.Synchronizer; import java.io.Serializable; import java.security.Principal; @@ -198,16 +199,13 @@ public boolean isReassignable() { * @param actor the actor to reassign the Decision to */ public final void reassign( final Principal actor ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { if( isReassignable() ) { m_actor = actor; } else { throw new IllegalArgumentException( "Decision cannot be reassigned." ); } - } finally { - lock.unlock(); - } + }); } /** diff --git a/jspwiki-main/src/main/java/org/apache/wiki/workflow/DecisionQueue.java b/jspwiki-main/src/main/java/org/apache/wiki/workflow/DecisionQueue.java index 592c3eb533..2847797b91 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/workflow/DecisionQueue.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/workflow/DecisionQueue.java @@ -23,6 +23,7 @@ Licensed to the Apache Software Foundation (ASF) under one import org.apache.wiki.api.exceptions.WikiException; import org.apache.wiki.event.WikiEventEmitter; import org.apache.wiki.event.WorkflowEvent; +import org.apache.wiki.util.Synchronizer; import java.io.Serializable; import java.security.Principal; @@ -66,13 +67,10 @@ public DecisionQueue() { * @param decision the Decision to add */ protected void add( final Decision decision ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { m_queue.addLast( decision ); decision.setId( next.getAndIncrement() ); - } finally { - lock.unlock(); - } + }); } /** @@ -91,12 +89,9 @@ protected Decision[] decisions() { * @param decision the decision to remove */ protected void remove( final Decision decision ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { m_queue.remove( decision ); - } finally { - lock.unlock(); - } + }); } /** @@ -155,8 +150,7 @@ public void decide( final Decision decision, final Outcome outcome, final Contex * @throws WikiException never */ public void reassign( final Decision decision, final Principal owner ) throws WikiException { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { if( decision.isReassignable() ) { decision.reassign( owner ); @@ -164,9 +158,7 @@ public void reassign( final Decision decision, final Principal owner ) throws Wi return; } throw new IllegalStateException( "Reassignments not allowed for this decision." ); - } finally { - lock.unlock(); - } + }); } } diff --git a/jspwiki-main/src/main/java/org/apache/wiki/workflow/DefaultWorkflowManager.java b/jspwiki-main/src/main/java/org/apache/wiki/workflow/DefaultWorkflowManager.java index 73a4dac064..bd94666fb0 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/workflow/DefaultWorkflowManager.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/workflow/DefaultWorkflowManager.java @@ -31,6 +31,7 @@ Licensed to the Apache Software Foundation (ASF) under one import org.apache.wiki.event.WikiEvent; import org.apache.wiki.event.WikiEventEmitter; import org.apache.wiki.event.WorkflowEvent; +import org.apache.wiki.util.Synchronizer; import org.apache.wiki.util.TextUtil; import java.io.*; @@ -153,8 +154,7 @@ public void initialize( final Engine engine, final Properties props ) { */ @SuppressWarnings( "unchecked" ) long unserializeFromDisk( final File f ) { - lock.lock(); - try { + return Synchronizer.synchronize(lock, () -> { long saved = 0L; final StopWatch sw = new StopWatch(); sw.start(); @@ -176,17 +176,14 @@ long unserializeFromDisk( final File f ) { sw.stop(); return saved; - } finally { - lock.unlock(); - } + }); } /** * Serializes workflows and decisionqueue to disk. The format is private, don't touch it. */ void serializeToDisk( final File f ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { try( final ObjectOutputStream out = new ObjectOutputStream( new BufferedOutputStream( Files.newOutputStream( f.toPath() ) ) ) ) { final StopWatch sw = new StopWatch(); sw.start(); @@ -203,9 +200,7 @@ void serializeToDisk( final File f ) { } catch( final IOException ioe ) { LOG.error( "Unable to serialize!", ioe ); } - } finally { - lock.unlock(); - } + }); } /** diff --git a/jspwiki-main/src/main/java/org/apache/wiki/workflow/Task.java b/jspwiki-main/src/main/java/org/apache/wiki/workflow/Task.java index 9eed1d90fc..9552fac4f0 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/workflow/Task.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/workflow/Task.java @@ -18,6 +18,8 @@ Licensed to the Apache Software Foundation (ASF) under one */ package org.apache.wiki.workflow; +import org.apache.wiki.util.Synchronizer; + import java.io.Serializable; import java.security.Principal; import java.util.Map; @@ -92,12 +94,9 @@ public final Principal getActor() { * @param step the successor */ public final void setSuccessor( final Step step ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { m_successor = step; - } finally { - lock.unlock(); - } + }); } /** @@ -107,12 +106,7 @@ public final void setSuccessor( final Step step ) { * @return the next step */ public final Step getSuccessor() { - lock.lock(); - try { - return m_successor; - } finally { - lock.unlock(); - } + return Synchronizer.synchronize(lock, () -> m_successor); } } diff --git a/jspwiki-main/src/main/java/org/apache/wiki/workflow/Workflow.java b/jspwiki-main/src/main/java/org/apache/wiki/workflow/Workflow.java index 55848038c3..daa627fb09 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/workflow/Workflow.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/workflow/Workflow.java @@ -22,6 +22,7 @@ Licensed to the Apache Software Foundation (ASF) under one import org.apache.wiki.api.exceptions.WikiException; import org.apache.wiki.event.WikiEventEmitter; import org.apache.wiki.event.WorkflowEvent; +import org.apache.wiki.util.Synchronizer; import java.io.IOException; import java.io.ObjectInputStream; @@ -247,8 +248,7 @@ public Workflow( final String messageKey, final Principal owner ) { * If the Workflow had been previously aborted, this method throws an IllegalStateException. */ public final void abort( final Context context ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { // Check corner cases: previous abort or completion if( m_state == ABORTED ) { throw new IllegalStateException( "The workflow has already been aborted." ); @@ -267,9 +267,7 @@ public final void abort( final Context context ) { m_state = ABORTED; WikiEventEmitter.fireWorkflowEvent( this, WorkflowEvent.ABORTED ); cleanup(); - } finally { - lock.unlock(); - } + }); } /** @@ -294,15 +292,12 @@ public final void addMessageArgument( final Serializable obj ) { * @return the current actor */ public final Principal getCurrentActor() { - lock.lock(); - try { + return Synchronizer.synchronize(lock, () -> { if( m_currentStep == null ) { return null; } return m_currentStep.getActor(); - } finally { - lock.unlock(); - } + }); } /** @@ -368,12 +363,7 @@ public final Date getEndTime() { */ public final int getId() { - lock.lock(); - try { - return m_id; - } finally { - lock.unlock(); - } + return Synchronizer.synchronize(lock, () -> m_id); } /** @@ -463,13 +453,10 @@ public final boolean isAborted() * @return true if the workflow has been started but has no more steps to perform; false if not. */ public final boolean isCompleted() { - lock.lock(); - try { + return Synchronizer.synchronize(lock, () -> { // If current step is null, then we're done return m_started && m_state == COMPLETED; - } finally { - lock.unlock(); - } + }); } /** @@ -502,8 +489,7 @@ public final Step getPreviousStep() * @throws WikiException if the current task's {@link Task#execute( Context )} method throws an exception */ public final void restart( final Context context ) throws WikiException { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { if( m_state != WAITING ) { throw new IllegalStateException( "Workflow is not paused; cannot restart." ); } @@ -518,9 +504,7 @@ public final void restart( final Context context ) throws WikiException { abort( context ); throw e; } - } finally { - lock.unlock(); - } + }); } /** @@ -545,12 +529,9 @@ public final void setAttribute( final String attr, final Serializable obj ) { */ public final void setFirstStep( final Step step ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { m_firstStep = step; - } finally { - lock.unlock(); - } + }); } /** @@ -560,12 +541,9 @@ public final void setFirstStep( final Step step ) */ public final void setId( final int id ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { this.m_id = id; - } finally { - lock.unlock(); - } + }); } /** @@ -577,8 +555,7 @@ public final void setId( final int id ) * @throws WikiException if the current Step's {@link Step#start()} method throws an exception of any kind */ public final void start( final Context context ) throws WikiException { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { if( m_state == ABORTED ) { throw new IllegalStateException( "Workflow cannot be started; it has already been aborted." ); } @@ -601,9 +578,7 @@ public final void start( final Context context ) throws WikiException { abort( context ); throw e; } - } finally { - lock.unlock(); - } + }); } /** @@ -611,16 +586,13 @@ public final void start( final Context context ) throws WikiException { * IllegalStateException. Once paused, the Workflow can be un-paused by executing the {@link #restart(Context)} method. */ public final void waitstate() { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { if ( m_state != RUNNING ) { throw new IllegalStateException( "Workflow is not running; cannot pause." ); } m_state = WAITING; WikiEventEmitter.fireWorkflowEvent( this, WorkflowEvent.WAITING ); - } finally { - lock.unlock(); - } + }); } /** @@ -636,16 +608,13 @@ protected void cleanup() { * calls the {@link #cleanup()} method to flush retained objects. This method will no-op if it has previously been called. */ protected final void complete() { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { if( !isCompleted() ) { m_state = COMPLETED; WikiEventEmitter.fireWorkflowEvent( this, WorkflowEvent.COMPLETED ); cleanup(); } - } finally { - lock.unlock(); - } + }); } /** diff --git a/jspwiki-util/src/main/java/org/apache/wiki/util/CommentedProperties.java b/jspwiki-util/src/main/java/org/apache/wiki/util/CommentedProperties.java index 5e207e5412..565665bf92 100644 --- a/jspwiki-util/src/main/java/org/apache/wiki/util/CommentedProperties.java +++ b/jspwiki-util/src/main/java/org/apache/wiki/util/CommentedProperties.java @@ -78,16 +78,13 @@ public CommentedProperties(final Properties defaultValues ) @Override public void load(final InputStream inStream ) throws IOException { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { // Load the file itself into a string m_propertyString = FileUtil.readContents( inStream, StandardCharsets.ISO_8859_1.name() ); // Now load it into the properties object as normal super.load( new ByteArrayInputStream( m_propertyString.getBytes(StandardCharsets.ISO_8859_1) ) ); - } finally { - lock.unlock(); - } + }); } /** @@ -99,15 +96,12 @@ public void load(final InputStream inStream ) throws IOException @Override public void load(final Reader in ) throws IOException { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { m_propertyString = FileUtil.readContents( in ); // Now load it into the properties object as normal super.load( new ByteArrayInputStream( m_propertyString.getBytes(StandardCharsets.ISO_8859_1) ) ); - } finally { - lock.unlock(); - } + }); } /** @@ -116,12 +110,7 @@ public void load(final Reader in ) throws IOException @Override public Object setProperty(final String key, final String value ) { - lock.lock(); - try { - return put(key, value); - } finally { - lock.unlock(); - } + return Synchronizer.synchronize(lock, () -> put(key, value)); } /** @@ -130,14 +119,11 @@ public Object setProperty(final String key, final String value ) @Override public void store(final OutputStream out, final String comments ) throws IOException { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { final byte[] bytes = m_propertyString.getBytes( StandardCharsets.ISO_8859_1 ); FileUtil.copyContents( new ByteArrayInputStream( bytes ), out ); out.flush(); - } finally { - lock.unlock(); - } + }); } /** @@ -146,16 +132,13 @@ public void store(final OutputStream out, final String comments ) throws IOExcep @Override public Object put(final Object arg0, final Object arg1 ) { - lock.lock(); - try { + return Synchronizer.synchronize(lock, () -> { // Write the property to the stored string writeProperty( arg0, arg1 ); // Return the result of from the superclass properties object return super.put(arg0, arg1); - } finally { - lock.unlock(); - } + }); } /** @@ -164,8 +147,7 @@ public Object put(final Object arg0, final Object arg1 ) @Override public void putAll(final Map< ? , ? > arg0 ) { - lock.lock(); - try { + Synchronizer.synchronize(lock, () -> { // Shove all of the entries into the property string for (final Entry value : arg0.entrySet()) { @SuppressWarnings("unchecked") final Entry entry = (Entry) value; @@ -174,9 +156,7 @@ public void putAll(final Map< ? , ? > arg0 ) // Call the superclass method super.putAll(arg0); - } finally { - lock.unlock(); - } + }); } /** @@ -185,16 +165,13 @@ public void putAll(final Map< ? , ? > arg0 ) @Override public Object remove(final Object key ) { - lock.lock(); - try { + return Synchronizer.synchronize(lock, () -> { // Remove from the property string deleteProperty( key ); // Call the superclass method return super.remove(key); - } finally { - lock.unlock(); - } + }); } /** @@ -203,12 +180,7 @@ public Object remove(final Object key ) @Override public String toString() { - lock.lock(); - try { - return m_propertyString; - } finally { - lock.unlock(); - } + return Synchronizer.synchronize(lock, () -> m_propertyString); } diff --git a/jspwiki-util/src/main/java/org/apache/wiki/util/Synchronizer.java b/jspwiki-util/src/main/java/org/apache/wiki/util/Synchronizer.java new file mode 100644 index 0000000000..2d59084883 --- /dev/null +++ b/jspwiki-util/src/main/java/org/apache/wiki/util/Synchronizer.java @@ -0,0 +1,150 @@ +/* + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. + */ + +package org.apache.wiki.util; + +import java.util.concurrent.locks.ReentrantLock; +import java.util.function.Supplier; + +/** + *

Synchronizer Utility Class

+ * + *

This utility class is designed to provide a simplified interface for + * executing code blocks in a synchronized manner using {@link ReentrantLock}. + * It aims to improve code readability and maintainability by abstracting + * common locking idioms.

+ * + *

Usage Example:

+ * 
+ * {@code
+ * ReentrantLock lock = new ReentrantLock();
+ * String result = Synchronizer.synchronize(lock, () -> {
+ *     // Your synchronized code here
+ *     return "some result";
+ * });
+ * }
+ *
+ * + * @since 2.12.2 + */ +public class Synchronizer { + + /** + * Executes a given {@link Supplier} within a synchronized block managed by + * a {@link ReentrantLock}. + * + *

This method acquires the lock, executes the supplier's code, and then + * releases the lock. It is designed to replace the traditional lock idiom:

+ * + * 
+     * {@code
+     * lock.lock();
+     * try {
+     *     doSomething();
+     * } finally {
+     *     lock.unlock();
+     * }
+     * }
+     *
+ * + *

Parameters:

+ *
    + *
  • {@code lock} - The ReentrantLock to be used for synchronization.
    • + *
  • {@code supplier} - The supplier whose code needs to be executed within + * the synchronized block.
    • + *
+ * + *

Returns:

+ *

The result produced by the supplier.

+ * + *

Throws:

+ *

This method propagates any exceptions thrown by the supplier's code.

+ * + * @param The type of result produced by the supplier. + * @param lock The ReentrantLock to be used for synchronization. + * @param supplier The supplier to be executed within the synchronized block. + * @return The result produced by the supplier. + */ + public static T synchronize(final ReentrantLock lock, final Supplier supplier) { + lock.lock(); + try { + return supplier.get(); + } finally { + lock.unlock(); + } + } + + /** + *

Functional interface for runnable tasks that can throw exceptions.

+ * + * @param the type of exception that may be thrown + */ + @FunctionalInterface + public interface ThrowingRunnable { + /** + * Executes the operation. + * + * @throws E if an exception occurs during the operation + */ + void run() throws E; + } + + /** + *

Throws the given exception as an unchecked exception.

+ * + * @param the type of exception to throw + * @param exception the exception to throw + * @throws E the thrown exception + */ + @SuppressWarnings("unchecked") + private static void throwAsUnchecked(final Exception exception) throws E { + throw (E) exception; + } + + /** + *

Executes a given {@link ThrowingRunnable} within a synchronized block managed by + * a {@link ReentrantLock}.

+ * + *

Parameters:

+ *
    + *
  • {@code lock} - The ReentrantLock to be used for synchronization.
    • + *
  • {@code throwingRunnable} - The ThrowingRunnable whose code needs to be executed within + * the synchronized block.
    • + *
+ * + *

Throws:

+ *

This method propagates any exceptions thrown by the ThrowingRunnable's code.

+ * + * @param the type of exception that may be thrown + * @param lock the ReentrantLock to use for synchronization + * @param throwingRunnable the ThrowingRunnable to execute + */ + public static void synchronize(final ReentrantLock lock, final ThrowingRunnable throwingRunnable) { + lock.lock(); + try { + throwingRunnable.run(); + } catch (final Exception e) { + throwAsUnchecked(e); + } finally { + lock.unlock(); + } + } + +} + diff --git a/pom.xml b/pom.xml index 1dfa3aee4c..0fe688d14c 100644 --- a/pom.xml +++ b/pom.xml @@ -458,6 +458,13 @@ javax.servlet-api ${javax-servlet-api.version} + + + org.apache.jspwiki + jspwiki-util + ${project.version} + +