Repository: tapestry-5 Updated Branches: refs/heads/master 57a8d25dd -> 325716199
TAP5-2406: Use a single ReentrantLock for the PeriodicExecutor and its jobs Project: http://git-wip-us.apache.org/repos/asf/tapestry-5/repo Commit: http://git-wip-us.apache.org/repos/asf/tapestry-5/commit/32571619 Tree: http://git-wip-us.apache.org/repos/asf/tapestry-5/tree/32571619 Diff: http://git-wip-us.apache.org/repos/asf/tapestry-5/diff/32571619 Branch: refs/heads/master Commit: 3257161990f639eabb60ce8729e3c1244823609a Parents: 57a8d25 Author: Howard M. Lewis Ship <hls...@apache.org> Authored: Fri Oct 24 13:59:37 2014 -0700 Committer: Howard M. Lewis Ship <hls...@apache.org> Committed: Fri Oct 24 13:59:37 2014 -0700 ---------------------------------------------------------------------- .../services/cron/PeriodicExecutorImpl.java | 181 +++++++++++++------ 1 file changed, 121 insertions(+), 60 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/32571619/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/cron/PeriodicExecutorImpl.java ---------------------------------------------------------------------- diff --git a/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/cron/PeriodicExecutorImpl.java b/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/cron/PeriodicExecutorImpl.java index abfcde9..cf3c717 100644 --- a/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/cron/PeriodicExecutorImpl.java +++ b/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/cron/PeriodicExecutorImpl.java @@ -1,5 +1,3 @@ -// Copyright 2011 The Apache Software Foundation -// // Licensed 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 @@ -26,6 +24,8 @@ import org.slf4j.Logger; import java.util.List; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; public class PeriodicExecutorImpl implements PeriodicExecutor, Runnable { @@ -33,18 +33,19 @@ public class PeriodicExecutorImpl implements PeriodicExecutor, Runnable private final Logger logger; - // Synchronized by this + // Synchronized by jobLock private final List<Job> jobs = CollectionFactory.newList(); private final Thread thread = new Thread(this, "Tapestry PeriodicExecutor"); - // Synchronized by this. Set when the registry is shutdown. - private boolean shutdown; + private transient boolean shutdown; private static final long FIVE_MINUTES = 5 * 60 * 1000; private final AtomicInteger jobIdAllocator = new AtomicInteger(); + private final Lock jobLock = new ReentrantLock(); + private class Job implements PeriodicJob, Invokable<Void> { final int jobId = jobIdAllocator.incrementAndGet(); @@ -74,43 +75,71 @@ public class PeriodicExecutorImpl implements PeriodicExecutor, Runnable return name; } - public synchronized long getNextExecution() + public long getNextExecution() { - return nextExecution; + try + { + jobLock.lock(); + return nextExecution; + } finally + { + jobLock.unlock(); + } } @Override - public synchronized boolean isExecuting() + public boolean isExecuting() { - return executing; + try + { + jobLock.lock(); + return executing; + } finally + { + jobLock.unlock(); + } } @Override - public synchronized boolean isCanceled() + public boolean isCanceled() { - return canceled; + try + { + jobLock.lock(); + return canceled; + } finally + { + jobLock.unlock(); + } } @Override - public synchronized void cancel() + public void cancel() { - canceled = true; + try + { + jobLock.lock(); + + canceled = true; + + if (!executing) + { + removeJob(this); + } - if (!executing) + // Otherwise, it will be caught when the job finishes execution. + } finally { - removeJob(this); + jobLock.unlock(); } - - // Otherwise, it will be caught when the job finishes execution. } @Override - public synchronized String toString() + public String toString() { StringBuilder builder = new StringBuilder("PeriodicJob[#").append(jobId); - builder.append(", (").append(name).append(")"); if (executing) @@ -133,17 +162,24 @@ public class PeriodicExecutorImpl implements PeriodicExecutor, Runnable * Starts execution of the job; this sets the executing flag, calculates the next execution time, * and uses the ParallelExecutor to run the job. */ - synchronized void start() + void start() { - executing = true; + try + { + jobLock.lock(); + executing = true; - // This is a bit naive; it assumes there will not be a delay waiting to execute. There's a lot of options - // here, such as basing the next execution on the actual start time, or event actual completion time, or allowing - // overlapping executions of the Job on a more rigid schedule. Use Quartz. + // This is a bit naive; it assumes there will not be a delay waiting to execute. There's a lot of options + // here, such as basing the next execution on the actual start time, or event actual completion time, or allowing + // overlapping executions of the Job on a more rigid schedule. Use Quartz. - nextExecution = schedule.nextExecution(nextExecution); + nextExecution = schedule.nextExecution(nextExecution); - parallelExecutor.invoke(this); + parallelExecutor.invoke(this); + } finally + { + jobLock.unlock(); + } if (logger.isTraceEnabled()) { @@ -151,22 +187,28 @@ public class PeriodicExecutorImpl implements PeriodicExecutor, Runnable } } - synchronized void cleanupAfterExecution() + void cleanupAfterExecution() { - if (logger.isTraceEnabled()) + try { - logger.trace(this + " execution complete"); - } + if (logger.isTraceEnabled()) + { + logger.trace(this + " execution complete"); + } - executing = false; + executing = false; - if (canceled) - { - removeJob(this); - } else + if (canceled) + { + removeJob(this); + } else + { + // Again, naive but necessary. + thread.interrupt(); + } + } finally { - // Again, naive but necessary. - thread.interrupt(); + jobLock.unlock(); } } @@ -188,6 +230,7 @@ public class PeriodicExecutorImpl implements PeriodicExecutor, Runnable return null; } + } public PeriodicExecutorImpl(ParallelExecutor parallelExecutor, Logger logger) @@ -212,19 +255,26 @@ public class PeriodicExecutorImpl implements PeriodicExecutor, Runnable } - synchronized void removeJob(Job job) + void removeJob(Job job) { if (logger.isDebugEnabled()) { logger.debug("Removing " + job); } - jobs.remove(job); + try + { + jobLock.lock(); + jobs.remove(job); + } finally + { + jobLock.unlock(); + } } @Override - public synchronized PeriodicJob addJob(Schedule schedule, String name, Runnable job) + public PeriodicJob addJob(Schedule schedule, String name, Runnable job) { assert schedule != null; assert name != null; @@ -232,7 +282,15 @@ public class PeriodicExecutorImpl implements PeriodicExecutor, Runnable Job periodicJob = new Job(schedule, name, job); - jobs.add(periodicJob); + try + { + jobLock.lock(); + + jobs.add(periodicJob); + } finally + { + jobLock.unlock(); + } if (logger.isDebugEnabled()) { @@ -252,7 +310,7 @@ public class PeriodicExecutorImpl implements PeriodicExecutor, Runnable @Override public void run() { - while (!isShutdown()) + while (!shutdown) { long nextExecution = executeCurrentBatch(); @@ -280,12 +338,7 @@ public class PeriodicExecutorImpl implements PeriodicExecutor, Runnable } } - private synchronized boolean isShutdown() - { - return shutdown; - } - - private synchronized void registryDidShutdown() + private void registryDidShutdown() { shutdown = true; @@ -297,27 +350,35 @@ public class PeriodicExecutorImpl implements PeriodicExecutor, Runnable * * @return the next execution time (from the non-executing job that is scheduled earliest for execution). */ - private synchronized long executeCurrentBatch() + private long executeCurrentBatch() { long now = System.currentTimeMillis(); long nextExecution = now + FIVE_MINUTES; - for (Job job : jobs) + try { - if (job.isExecuting()) + jobLock.lock(); + + for (Job job : jobs) { - continue; - } + if (job.isExecuting()) + { + continue; + } - long jobNextExecution = job.getNextExecution(); + long jobNextExecution = job.getNextExecution(); - if (jobNextExecution <= now) - { - job.start(); - } else - { - nextExecution = Math.min(nextExecution, jobNextExecution); + if (jobNextExecution <= now) + { + job.start(); + } else + { + nextExecution = Math.min(nextExecution, jobNextExecution); + } } + } finally + { + jobLock.unlock(); } return nextExecution;