diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/SourceCodeProcessor.java b/pmd-core/src/main/java/net/sourceforge/pmd/SourceCodeProcessor.java index a658f66feb..346c3972d5 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/SourceCodeProcessor.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/SourceCodeProcessor.java @@ -90,6 +90,7 @@ public class SourceCodeProcessor { } try { + ruleSets.start(ctx); processSource(sourceCode, ruleSets, ctx); } catch (ParseException pe) { configuration.getAnalysisCache().analysisFailed(ctx.getSourceCodeFile()); @@ -99,6 +100,7 @@ public class SourceCodeProcessor { throw new PMDException("Error while processing " + ctx.getSourceCodeFilename(), e); } finally { IOUtils.closeQuietly(sourceCode); + ruleSets.end(ctx); } } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/processor/AbstractPMDProcessor.java b/pmd-core/src/main/java/net/sourceforge/pmd/processor/AbstractPMDProcessor.java index 1f1456bdbb..b16491a759 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/processor/AbstractPMDProcessor.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/processor/AbstractPMDProcessor.java @@ -9,9 +9,11 @@ import java.util.List; import net.sourceforge.pmd.PMDConfiguration; import net.sourceforge.pmd.Report; +import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.RuleSetFactory; import net.sourceforge.pmd.RuleSets; import net.sourceforge.pmd.RulesetsFactoryUtils; +import net.sourceforge.pmd.SourceCodeProcessor; import net.sourceforge.pmd.benchmark.Benchmark; import net.sourceforge.pmd.benchmark.Benchmarker; import net.sourceforge.pmd.renderers.Renderer; @@ -61,4 +63,23 @@ public abstract class AbstractPMDProcessor { return RulesetsFactoryUtils.getRuleSets(configuration.getRuleSets(), factory); } + public void processFiles(RuleSetFactory ruleSetFactory, List files, RuleContext ctx, + List renderers) { + RuleSets rs = createRuleSets(ruleSetFactory); + configuration.getAnalysisCache().checkValidity(rs, configuration.getClassLoader()); + SourceCodeProcessor processor = new SourceCodeProcessor(configuration); + + for (DataSource dataSource : files) { + String niceFileName = filenameFrom(dataSource); + + runAnalysis(new PmdRunnable(configuration, dataSource, niceFileName, renderers, + ctx, ruleSetFactory, processor)); + } + + collectReports(renderers); + } + + protected abstract void runAnalysis(PmdRunnable runnable); + + protected abstract void collectReports(List renderers); } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/processor/MonoThreadProcessor.java b/pmd-core/src/main/java/net/sourceforge/pmd/processor/MonoThreadProcessor.java index 5cee8bcfc0..1adb114a89 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/processor/MonoThreadProcessor.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/processor/MonoThreadProcessor.java @@ -4,23 +4,12 @@ package net.sourceforge.pmd.processor; -import java.io.BufferedInputStream; -import java.io.IOException; -import java.io.InputStream; +import java.util.ArrayList; import java.util.List; -import java.util.logging.Level; -import java.util.logging.Logger; -import net.sourceforge.pmd.PMD; import net.sourceforge.pmd.PMDConfiguration; -import net.sourceforge.pmd.PMDException; import net.sourceforge.pmd.Report; -import net.sourceforge.pmd.RuleContext; -import net.sourceforge.pmd.RuleSetFactory; -import net.sourceforge.pmd.RuleSets; -import net.sourceforge.pmd.SourceCodeProcessor; import net.sourceforge.pmd.renderers.Renderer; -import net.sourceforge.pmd.util.datasource.DataSource; /** * @author Romain Pelisse <belaran@gmail.com> @@ -28,60 +17,25 @@ import net.sourceforge.pmd.util.datasource.DataSource; */ public final class MonoThreadProcessor extends AbstractPMDProcessor { - private static final Logger LOG = Logger.getLogger(MonoThreadProcessor.class.getName()); - + private final List reports = new ArrayList<>(); + public MonoThreadProcessor(PMDConfiguration configuration) { super(configuration); } - public void processFiles(RuleSetFactory ruleSetFactory, List files, RuleContext ctx, - List renderers) { - - // single threaded execution - - RuleSets rs = createRuleSets(ruleSetFactory); - configuration.getAnalysisCache().checkValidity(rs, configuration.getClassLoader()); - SourceCodeProcessor processor = new SourceCodeProcessor(configuration); - - for (DataSource dataSource : files) { - String niceFileName = filenameFrom(dataSource); - - Report report = PMD.setupReport(rs, ctx, niceFileName); - - if (LOG.isLoggable(Level.FINE)) { - LOG.fine("Processing " + ctx.getSourceCodeFilename()); - } - rs.start(ctx); - - for (Renderer r : renderers) { - r.startFileAnalysis(dataSource); - } - - try { - InputStream stream = new BufferedInputStream(dataSource.getInputStream()); - ctx.setLanguageVersion(null); - processor.processSourceCode(stream, rs, ctx); - } catch (PMDException pmde) { - if (LOG.isLoggable(Level.FINE)) { - LOG.log(Level.FINE, "Error while processing file: " + niceFileName, pmde.getCause()); - } - - report.addError(new Report.ProcessingError(pmde.getMessage(), niceFileName)); - } catch (IOException ioe) { - // unexpected exception: log and stop executor service - addError(report, "Unable to read source file", ioe, niceFileName); - } catch (RuntimeException re) { - // unexpected exception: log and stop executor service - addError(report, "RuntimeException while processing file", re, niceFileName); - } - - rs.end(ctx); - super.renderReports(renderers, ctx.getReport()); - } + @Override + protected void runAnalysis(PmdRunnable runnable) { + // single thread execution, run analysis on same thread + reports.add(runnable.call()); } - private void addError(Report report, String msg, Exception ex, String fileName) { - LOG.log(Level.FINE, msg, ex); - report.addError(new Report.ProcessingError(ex.getMessage(), fileName)); + @Override + protected void collectReports(List renderers) { + for (Report r : reports) { + super.renderReports(renderers, r); + } + + // Since this thread may run PMD again, clean up the runnable + PmdRunnable.reset(); } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/processor/MultiThreadProcessor.java b/pmd-core/src/main/java/net/sourceforge/pmd/processor/MultiThreadProcessor.java index bd59f5684b..68a68b6f71 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/processor/MultiThreadProcessor.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/processor/MultiThreadProcessor.java @@ -6,18 +6,17 @@ package net.sourceforge.pmd.processor; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.CompletionService; import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorCompletionService; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; +import java.util.concurrent.ThreadFactory; import net.sourceforge.pmd.PMDConfiguration; import net.sourceforge.pmd.Report; -import net.sourceforge.pmd.RuleContext; -import net.sourceforge.pmd.RuleSetFactory; -import net.sourceforge.pmd.RuleSets; import net.sourceforge.pmd.renderers.Renderer; -import net.sourceforge.pmd.util.datasource.DataSource; /** * @author Romain Pelisse <belaran@gmail.com> @@ -25,62 +24,46 @@ import net.sourceforge.pmd.util.datasource.DataSource; */ public class MultiThreadProcessor extends AbstractPMDProcessor { + private ThreadFactory factory; + private ExecutorService executor; + private CompletionService completionService; + private List> tasks = new ArrayList<>(); + public MultiThreadProcessor(final PMDConfiguration configuration) { super(configuration); + + factory = new PmdThreadFactory(); + executor = Executors.newFixedThreadPool(configuration.getThreads(), factory); + completionService = new ExecutorCompletionService<>(executor); } - /** - * Run PMD on a list of files using multiple threads. - */ - public void processFiles(final RuleSetFactory ruleSetFactory, final List files, final RuleContext ctx, - final List renderers) { - - RuleSets rs = createRuleSets(ruleSetFactory); - rs.start(ctx); - configuration.getAnalysisCache().checkValidity(rs, configuration.getClassLoader()); - - PmdThreadFactory factory = new PmdThreadFactory(ruleSetFactory, ctx); - ExecutorService executor = Executors.newFixedThreadPool(configuration.getThreads(), factory); - List> tasks = new ArrayList<>(files.size()); - - for (DataSource dataSource : files) { - String niceFileName = filenameFrom(dataSource); - - PmdRunnable r = new PmdRunnable(executor, configuration, dataSource, niceFileName, renderers); - Future future = executor.submit(r); - tasks.add(future); - } - executor.shutdown(); - - processReports(renderers, tasks); - - rs.end(ctx); - super.renderReports(renderers, ctx.getReport()); - + @Override + protected void runAnalysis(PmdRunnable runnable) { + // multi-threaded execution, dispatch analysis to worker threads + tasks.add(completionService.submit(runnable)); } - private void processReports(final List renderers, List> tasks) { - - while (!tasks.isEmpty()) { - Future future = tasks.remove(0); - Report report = null; - try { - report = future.get(); - } catch (InterruptedException ie) { - Thread.currentThread().interrupt(); - future.cancel(true); - } catch (ExecutionException ee) { - Throwable t = ee.getCause(); - if (t instanceof RuntimeException) { - throw (RuntimeException) t; - } else if (t instanceof Error) { - throw (Error) t; - } else { - throw new IllegalStateException("PmdRunnable exception", t); - } + @Override + protected void collectReports(List renderers) { + // Collect result analysis, waiting for termination if needed + try { + for (int i = 0; i < tasks.size(); i++) { + final Report report = completionService.take().get(); + super.renderReports(renderers, report); } - - super.renderReports(renderers, report); + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + } catch (ExecutionException ee) { + Throwable t = ee.getCause(); + if (t instanceof RuntimeException) { + throw (RuntimeException) t; + } else if (t instanceof Error) { + throw (Error) t; + } else { + throw new IllegalStateException("PmdRunnable exception", t); + } + } finally { + executor.shutdownNow(); } } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/processor/PmdRunnable.java b/pmd-core/src/main/java/net/sourceforge/pmd/processor/PmdRunnable.java index eff91bd42b..a85d29511d 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/processor/PmdRunnable.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/processor/PmdRunnable.java @@ -9,7 +9,6 @@ import java.io.IOException; import java.io.InputStream; import java.util.List; import java.util.concurrent.Callable; -import java.util.concurrent.ExecutorService; import java.util.logging.Level; import java.util.logging.Logger; @@ -19,52 +18,65 @@ import net.sourceforge.pmd.PMDException; import net.sourceforge.pmd.Report; import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.RuleSetFactory; +import net.sourceforge.pmd.RuleSetNotFoundException; import net.sourceforge.pmd.RuleSets; +import net.sourceforge.pmd.SourceCodeProcessor; import net.sourceforge.pmd.renderers.Renderer; import net.sourceforge.pmd.util.datasource.DataSource; -public class PmdRunnable extends PMD implements Callable { +public class PmdRunnable implements Callable { private static final Logger LOG = Logger.getLogger(PmdRunnable.class.getName()); - private final ExecutorService executor; + private static final ThreadLocal LOCAL_THREAD_CONTEXT = new ThreadLocal<>(); + + private final PMDConfiguration configuration; private final DataSource dataSource; private final String fileName; private final List renderers; + private final RuleContext ruleContext; + private final RuleSetFactory ruleSetFactory; + private final SourceCodeProcessor sourceCodeProcessor; - public PmdRunnable(ExecutorService executor, PMDConfiguration configuration, DataSource dataSource, String fileName, - List renderers) { - super(configuration); - this.executor = executor; + public PmdRunnable(PMDConfiguration configuration, DataSource dataSource, String fileName, + List renderers, RuleContext ruleContext, RuleSetFactory ruleSetFactory, + SourceCodeProcessor sourceCodeProcessor) { + this.configuration = configuration; this.dataSource = dataSource; this.fileName = fileName; this.renderers = renderers; + this.ruleContext = ruleContext; + this.ruleSetFactory = ruleSetFactory; + this.sourceCodeProcessor = sourceCodeProcessor; } - // If we ever end up having a ReportUtil class, this method should be moved - // there... - private static void addError(Report report, Exception ex, String fileName) { - report.addError(new Report.ProcessingError(ex.getMessage(), fileName)); + public static void reset() { + LOCAL_THREAD_CONTEXT.remove(); } - private void addErrorAndShutdown(Report report, Exception e, String errorMessage) { + private void addError(Report report, Exception e, String errorMessage) { // unexpected exception: log and stop executor service LOG.log(Level.FINE, errorMessage, e); - addError(report, e, fileName); - executor.shutdownNow(); + report.addError(new Report.ProcessingError(e.getMessage(), fileName)); } @Override public Report call() { - PmdThread thread = (PmdThread) Thread.currentThread(); + ThreadContext tc = LOCAL_THREAD_CONTEXT.get(); + if (tc == null) { + try { + tc = new ThreadContext(ruleSetFactory.createRuleSets(configuration.getRuleSets()), + new RuleContext(ruleContext)); + } catch (RuleSetNotFoundException e) { + throw new RuntimeException(e); + } + LOCAL_THREAD_CONTEXT.set(tc); + } - RuleContext ctx = thread.getRuleContext(); - RuleSets rs = thread.getRuleSets(configuration.getRuleSets()); - - Report report = setupReport(rs, ctx, fileName); + Report report = PMD.setupReport(tc.ruleSets, tc.ruleContext, fileName); if (LOG.isLoggable(Level.FINE)) { - LOG.fine("Processing " + ctx.getSourceCodeFilename()); + LOG.fine("Processing " + tc.ruleContext.getSourceCodeFilename()); } for (Renderer r : renderers) { r.startFileAnalysis(dataSource); @@ -72,59 +84,26 @@ public class PmdRunnable extends PMD implements Callable { try { InputStream stream = new BufferedInputStream(dataSource.getInputStream()); - ctx.setLanguageVersion(null); - this.getSourceCodeProcessor().processSourceCode(stream, rs, ctx); + tc.ruleContext.setLanguageVersion(null); + sourceCodeProcessor.processSourceCode(stream, tc.ruleSets, tc.ruleContext); } catch (PMDException pmde) { - if (LOG.isLoggable(Level.FINE)) { - LOG.log(Level.FINE, "Error while processing file: " + fileName, pmde.getCause()); - } - addError(report, pmde, fileName); + addError(report, pmde, "Error while processing file: " + fileName); } catch (IOException ioe) { - addErrorAndShutdown(report, ioe, "IOException during processing of " + fileName); - + addError(report, ioe, "IOException during processing of " + fileName); } catch (RuntimeException re) { - addErrorAndShutdown(report, re, "RuntimeException during processing of " + fileName); + addError(report, re, "RuntimeException during processing of " + fileName); } + return report; } - private static class PmdThread extends Thread { + private static class ThreadContext { + /* default */ final RuleSets ruleSets; + /* default */ final RuleContext ruleContext; - private final int id; - private RuleContext context; - private RuleSets rulesets; - private final RuleSetFactory ruleSetFactory; - - PmdThread(int id, Runnable r, RuleSetFactory ruleSetFactory, RuleContext ctx) { - super(r, "PmdThread " + id); - this.id = id; - context = new RuleContext(ctx); - this.ruleSetFactory = ruleSetFactory; + ThreadContext(RuleSets ruleSets, RuleContext ruleContext) { + this.ruleSets = ruleSets; + this.ruleContext = ruleContext; } - - public RuleContext getRuleContext() { - return context; - } - - public RuleSets getRuleSets(String rsList) { - if (rulesets == null) { - try { - // this creates an own copy of the ruleset for this thread - rulesets = ruleSetFactory.createRuleSets(rsList); - } catch (Exception e) { - e.printStackTrace(); - } - } - return rulesets; - } - - @Override - public String toString() { - return "PmdThread " + id; - } - } - - public static Thread createThread(int id, Runnable r, RuleSetFactory ruleSetFactory, RuleContext ctx) { - return new PmdThread(id, r, ruleSetFactory, ctx); } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/processor/PmdThreadFactory.java b/pmd-core/src/main/java/net/sourceforge/pmd/processor/PmdThreadFactory.java index aa47e0dc5a..51c84b3063 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/processor/PmdThreadFactory.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/processor/PmdThreadFactory.java @@ -7,24 +7,13 @@ package net.sourceforge.pmd.processor; import java.util.concurrent.ThreadFactory; import java.util.concurrent.atomic.AtomicInteger; -import net.sourceforge.pmd.RuleContext; -import net.sourceforge.pmd.RuleSetFactory; - public class PmdThreadFactory implements ThreadFactory { - private final RuleSetFactory ruleSetFactory; - private final RuleContext ctx; private final AtomicInteger counter = new AtomicInteger(); - public PmdThreadFactory(RuleSetFactory ruleSetFactory, RuleContext ctx) { - this.ruleSetFactory = ruleSetFactory; - this.ctx = ctx; - } - @Override public Thread newThread(Runnable r) { - Thread t = PmdRunnable.createThread(counter.incrementAndGet(), r, ruleSetFactory, ctx); - return t; + return new Thread(r, "PmdThread " + counter.incrementAndGet()); } } diff --git a/pmd-test/src/main/java/net/sourceforge/pmd/testframework/RuleTst.java b/pmd-test/src/main/java/net/sourceforge/pmd/testframework/RuleTst.java index 32287bb2a4..2cb6434b43 100644 --- a/pmd-test/src/main/java/net/sourceforge/pmd/testframework/RuleTst.java +++ b/pmd-test/src/main/java/net/sourceforge/pmd/testframework/RuleTst.java @@ -219,9 +219,7 @@ public abstract class RuleTst { ctx.setLanguageVersion(languageVersion); ctx.setIgnoreExceptions(false); RuleSet rules = new RuleSetFactory().createSingleRuleRuleSet(rule); - rules.start(ctx); p.getSourceCodeProcessor().processSourceCode(new StringReader(code), new RuleSets(rules), ctx); - rules.end(ctx); } catch (Exception e) { throw new RuntimeException(e); } diff --git a/src/site/markdown/customizing/howtowritearule.md b/src/site/markdown/customizing/howtowritearule.md index 9731d8a9ad..ce5f6f906b 100644 --- a/src/site/markdown/customizing/howtowritearule.md +++ b/src/site/markdown/customizing/howtowritearule.md @@ -271,7 +271,7 @@ rule that checks stuff across the all source code? Let's take a dummy example. L rule that count how many Expression Node you have in your source code (told you, it was a dummy example :) ). You realize quite simply. You just have to add static field to the RulesContext, as an attribute, and uses -Rule.start() and Rule.end() hook to initialized and finalize your rule's implementation: +`Rule.start()` and `Rule.end()` hooks to initialize and finalize your rule's implementation: package net.sourceforge.pmd.rules; @@ -309,9 +309,9 @@ Rule.start() and Rule.end() hook to initialized and finalize your rule's impleme } } -As you can see in this example, the method start will be call the first time the rule is going to be used, -so you can initialize properly your rule here. Once the rule will have finished to parses the source code, -the method end() will be invoke you can assert there if, or not, your rule has been violated. +As you can see in this example, the method `start()` will be called once per file, right before it's analysis starts, +so you can initialize properly your rule here. Once the rule have finished analyzing the file's source code, +the method `end()` will be invoked you can assert there if your rule has been violated or not. Note that the example logs a violation **without** a proper classname. This is not really a good idea. diff --git a/src/site/markdown/overview/changelog.md b/src/site/markdown/overview/changelog.md index 6ec5a391ae..3760548d0a 100644 --- a/src/site/markdown/overview/changelog.md +++ b/src/site/markdown/overview/changelog.md @@ -228,6 +228,7 @@ to avoid XSS attacks. ### Fixed Issues * General + * [#1511](https://sourceforge.net/p/pmd/bugs/1511/): \[core] Inconsistent behavior of Rule.start/Rule.end * [#1542](https://sourceforge.net/p/pmd/bugs/1542/): \[java] CPD throws an NPE when parsing enums with -ignore-identifiers * apex-apexunit * [#1543](https://sourceforge.net/p/pmd/bugs/1543/): \[apex] ApexUnitTestClassShouldHaveAsserts assumes APEX is case sensitive