From 1c404b050df1825dfb29252517b58fbecada4211 Mon Sep 17 00:00:00 2001 From: Martin Weber Date: Thu, 10 Feb 2022 21:40:30 +0100 Subject: [PATCH] Bug 578683: o.e.cdt.jsoncdb.core Arglets are not thread safe Change-Id: I5f54d6729cffcf47382a37a436a2772a5ca84340 Signed-off-by: Martin Weber --- .../guide/deprecated_API_removals.html | 23 ++++ .../core/internal/ParserDetection.java | 5 +- .../cdt/jsoncdb/core/participant/Arglets.java | 37 +++--- .../DefaultToolDetectionParticipant.java | 113 +++++++++++------- .../IToolDetectionParticipant.java | 5 +- .../core/participant/ResponseFileArglets.java | 3 +- 6 files changed, 115 insertions(+), 71 deletions(-) diff --git a/doc/org.eclipse.cdt.doc.isv/guide/deprecated_API_removals.html b/doc/org.eclipse.cdt.doc.isv/guide/deprecated_API_removals.html index 175c0e490b8..461f30947bd 100644 --- a/doc/org.eclipse.cdt.doc.isv/guide/deprecated_API_removals.html +++ b/doc/org.eclipse.cdt.doc.isv/guide/deprecated_API_removals.html @@ -70,6 +70,12 @@
  • GnuMakefileGenerator is no longer part of API
  • The Spawner signal constants are nolonger API
    • +

    + Planned Removals after March 2024 +

    +
      +
    1. java.util.regex.Matcher use in JSONCDB API will be removed
      2. +

    API Changes prior to CDT 10.0 / 2020-09. @@ -524,6 +530,23 @@

  • INT
  • CTRLC
    • + + +

    API Removals after March 2024

    + +

    1. java.util.regex.Matcher use in JSONCDB API will be removed

    +

    + The following fields will be removed from the API as it is not thread safe. Use the patten instead and call matcher(input) to obtain a matcher. +

    + +

    + See Bug 578683. +

    + diff --git a/jsoncdb/org.eclipse.cdt.jsoncdb.core/src/org/eclipse/cdt/jsoncdb/core/internal/ParserDetection.java b/jsoncdb/org.eclipse.cdt.jsoncdb.core/src/org/eclipse/cdt/jsoncdb/core/internal/ParserDetection.java index 87326fa2fad..ec9d1d17eba 100644 --- a/jsoncdb/org.eclipse.cdt.jsoncdb.core/src/org/eclipse/cdt/jsoncdb/core/internal/ParserDetection.java +++ b/jsoncdb/org.eclipse.cdt.jsoncdb.core/src/org/eclipse/cdt/jsoncdb/core/internal/ParserDetection.java @@ -14,6 +14,7 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.function.Consumer; @@ -389,8 +390,8 @@ public final class ParserDetection { */ public ParserDetectionResult(DetectorWithMethod detectorWMethod, DefaultToolDetectionParticipant.MatchResult commandLine) { - this.detectorWMethod = detectorWMethod; - this.commandLine = commandLine; + this.detectorWMethod = Objects.requireNonNull(detectorWMethod, "detectorWMethod"); + this.commandLine = Objects.requireNonNull(commandLine, "commandLine"); } /** diff --git a/jsoncdb/org.eclipse.cdt.jsoncdb.core/src/org/eclipse/cdt/jsoncdb/core/participant/Arglets.java b/jsoncdb/org.eclipse.cdt.jsoncdb.core/src/org/eclipse/cdt/jsoncdb/core/participant/Arglets.java index c7d8573f169..1c76ef26931 100644 --- a/jsoncdb/org.eclipse.cdt.jsoncdb.core/src/org/eclipse/cdt/jsoncdb/core/participant/Arglets.java +++ b/jsoncdb/org.eclipse.cdt.jsoncdb.core/src/org/eclipse/cdt/jsoncdb/core/participant/Arglets.java @@ -62,7 +62,13 @@ public final class Arglets { * @author Martin Weber */ public static class NameOptionMatcher { + /** + * Do not use the matcher field, it is not thread safe. Instead use pattern.matcher(input). + * @deprecated + */ + @Deprecated(forRemoval = true) final Matcher matcher; + final Pattern pattern; final int nameGroup; /** @@ -72,14 +78,15 @@ public final class Arglets { * @param nameGroup - capturing group number defining name of an entry. */ public NameOptionMatcher(String pattern, int nameGroup) { - this.matcher = Pattern.compile(pattern).matcher(EMPTY_STR); + this.pattern = Pattern.compile(pattern); + this.matcher = this.pattern.matcher(EMPTY_STR); this.nameGroup = nameGroup; } @SuppressWarnings("nls") @Override public String toString() { - return "NameOptionMatcher [matcher=" + this.matcher + ", nameGroup=" + this.nameGroup + "]"; + return "NameOptionMatcher [pattern=" + this.pattern + ", nameGroup=" + this.nameGroup + "]"; } } @@ -99,13 +106,8 @@ public final class Arglets { /** * Constructor. * - * @param pattern - regular expression pattern being parsed by the parser. - * @param nameGroup - capturing group number defining name of an entry. - * @param valueGroup - capturing group number defining value of an entry. - */ - /** - * @param pattern - * @param nameGroup + * @param pattern regular expression pattern being parsed by the parser. + * @param nameGroup the number of the value group * @param valueGroup the number of the value group, or {@code -1} for a pattern * that does not recognize a macro value */ @@ -117,7 +119,7 @@ public final class Arglets { @SuppressWarnings("nls") @Override public String toString() { - return "NameValueOptionMatcher [matcher=" + this.matcher + ", nameGroup=" + this.nameGroup + ", valueGroup=" + return "NameValueOptionMatcher [pattern=" + this.pattern + ", nameGroup=" + this.nameGroup + ", valueGroup=" + this.valueGroup + "]"; } } @@ -134,9 +136,8 @@ public final class Arglets { protected final int processArgument(IArgumentCollector resultCollector, String args, NameValueOptionMatcher[] optionMatchers) { for (NameValueOptionMatcher oMatcher : optionMatchers) { - final Matcher matcher = oMatcher.matcher; + final Matcher matcher = oMatcher.pattern.matcher(args); - matcher.reset(args); if (matcher.lookingAt()) { final String name = matcher.group(oMatcher.nameGroup); final String value = oMatcher.valueGroup == -1 ? null : matcher.group(oMatcher.valueGroup); @@ -159,9 +160,8 @@ public final class Arglets { */ protected final int processArgument(IArgumentCollector resultCollector, String argsLine, NameOptionMatcher optionMatcher) { - final Matcher oMatcher = optionMatcher.matcher; + final Matcher oMatcher = optionMatcher.pattern.matcher(argsLine); - oMatcher.reset(argsLine); if (oMatcher.lookingAt()) { final String name = oMatcher.group(1); resultCollector.addUndefine(name); @@ -185,9 +185,8 @@ public final class Arglets { protected final int processArgument(boolean isSystemIncludePath, IArgumentCollector resultCollector, IPath cwd, String argsLine, NameOptionMatcher[] optionMatchers) { for (NameOptionMatcher oMatcher : optionMatchers) { - final Matcher matcher = oMatcher.matcher; + final Matcher matcher = oMatcher.pattern.matcher(argsLine); - matcher.reset(argsLine); if (matcher.lookingAt()) { String name = matcher.group(oMatcher.nameGroup); // workaround for relative path by cmake bug @@ -221,9 +220,8 @@ public final class Arglets { protected final int processArgument(IArgumentCollector resultCollector, IPath cwd, String argsLine, NameOptionMatcher[] optionMatchers) { for (NameOptionMatcher oMatcher : optionMatchers) { - final Matcher matcher = oMatcher.matcher; + final Matcher matcher = oMatcher.pattern.matcher(argsLine); - matcher.reset(argsLine); if (matcher.lookingAt()) { String name = matcher.group(oMatcher.nameGroup); resultCollector.addIncludeFile(name); @@ -246,9 +244,8 @@ public final class Arglets { protected final int processArgument(IArgumentCollector resultCollector, IPath cwd, String argsLine, NameOptionMatcher[] optionMatchers) { for (NameOptionMatcher oMatcher : optionMatchers) { - final Matcher matcher = oMatcher.matcher; + final Matcher matcher = oMatcher.pattern.matcher(argsLine); - matcher.reset(argsLine); if (matcher.lookingAt()) { String name = matcher.group(oMatcher.nameGroup); resultCollector.addMacroFile(name); diff --git a/jsoncdb/org.eclipse.cdt.jsoncdb.core/src/org/eclipse/cdt/jsoncdb/core/participant/DefaultToolDetectionParticipant.java b/jsoncdb/org.eclipse.cdt.jsoncdb.core/src/org/eclipse/cdt/jsoncdb/core/participant/DefaultToolDetectionParticipant.java index 30a4e1c3097..3b03265f0ee 100644 --- a/jsoncdb/org.eclipse.cdt.jsoncdb.core/src/org/eclipse/cdt/jsoncdb/core/participant/DefaultToolDetectionParticipant.java +++ b/jsoncdb/org.eclipse.cdt.jsoncdb.core/src/org/eclipse/cdt/jsoncdb/core/participant/DefaultToolDetectionParticipant.java @@ -71,19 +71,33 @@ public class DefaultToolDetectionParticipant implements IToolDetectionParticipan protected static final String REGEX_CMD_PATH_BSLASH_QUOTE_END = ")\\1\\s"; /** - * the Matchers to match the name of the tool (including its path, BUT WITHOUT + * the Patterns to match the name of the tool (including its path, BUT WITHOUT * its filename extension) on a given command-line */ - private final Matcher[] toolNameMatchers; - private final Matcher[] toolNameMatchersBackslash; + private final Pattern[] toolNamePatterns; + private final Pattern[] toolNamePatternsBackslash; /** * the Matcher that matches the name of the tool (including its path AND its * filename extension) on a given command-line or {@code null} + * + * @deprecated Use toolNamePatternsExt instead */ + @Deprecated(forRemoval = true) protected final Matcher[] toolNameMatchersExt; + /** + * @deprecated Use toolNamePatternsExtBackslash instead + */ + @Deprecated(forRemoval = true) protected final Matcher[] toolNameMatchersExtBackslash; + /** + * the Pattern that matches the name of the tool (including its path AND its + * filename extension) on a given command-line or {@code null} + */ + private final Pattern[] toolNamePatternsExt; + private final Pattern[] toolNamePatternsExtBackslash; + /** * the corresponding parser for the tool arguments */ @@ -174,39 +188,48 @@ public class DefaultToolDetectionParticipant implements IToolDetectionParticipan this.alsoHandleNtfsPaths = alsoHandleNtfsPaths; this.extensionRegex = extensionRegex; - this.toolNameMatchers = new Matcher[] { + this.toolNamePatterns = new Pattern[] { Pattern.compile(String.format(Locale.ROOT, "%s%s%s", REGEX_CMD_PATH_SLASH_QUOTE, basenameRegex, - REGEX_CMD_PATH_SLASH_QUOTE_END)).matcher(""), + REGEX_CMD_PATH_SLASH_QUOTE_END)), Pattern.compile(String.format(Locale.ROOT, "%s%s%s", REGEX_CMD_PATH_SLASH, basenameRegex, - REGEX_CMD_PATH_SLASH_END)).matcher("") }; + REGEX_CMD_PATH_SLASH_END)) }; if (alsoHandleNtfsPaths) { - this.toolNameMatchersBackslash = new Matcher[] { + this.toolNamePatternsBackslash = new Pattern[] { Pattern.compile(String.format(Locale.ROOT, "%s%s%s", REGEX_CMD_PATH_BSLASH_QUOTE, basenameRegex, - REGEX_CMD_PATH_BSLASH_QUOTE_END)).matcher(""), + REGEX_CMD_PATH_BSLASH_QUOTE_END)), Pattern.compile(String.format(Locale.ROOT, "%s%s%s", REGEX_CMD_PATH_BSLASH, basenameRegex, - REGEX_CMD_PATH_BSLASH_END)).matcher("") }; + REGEX_CMD_PATH_BSLASH_END)) }; } else { - this.toolNameMatchersBackslash = new Matcher[] {}; + this.toolNamePatternsBackslash = new Pattern[] {}; } if (extensionRegex != null) { - this.toolNameMatchersExt = new Matcher[] { + this.toolNamePatternsExt = new Pattern[] { Pattern.compile(String.format(Locale.ROOT, "%s%s\\.%s%s", REGEX_CMD_PATH_SLASH_QUOTE, basenameRegex, - extensionRegex, REGEX_CMD_PATH_SLASH_QUOTE_END)).matcher(""), + extensionRegex, REGEX_CMD_PATH_SLASH_QUOTE_END)), Pattern.compile(String.format(Locale.ROOT, "%s%s\\.%s%s", REGEX_CMD_PATH_SLASH, basenameRegex, - extensionRegex, REGEX_CMD_PATH_SLASH_END)).matcher("") }; + extensionRegex, REGEX_CMD_PATH_SLASH_END)) }; if (alsoHandleNtfsPaths) { - this.toolNameMatchersExtBackslash = new Matcher[] { + this.toolNamePatternsExtBackslash = new Pattern[] { Pattern.compile(String.format(Locale.ROOT, "%s%s\\.%s%s", REGEX_CMD_PATH_BSLASH_QUOTE, - basenameRegex, extensionRegex, REGEX_CMD_PATH_BSLASH_QUOTE_END)).matcher(""), + basenameRegex, extensionRegex, REGEX_CMD_PATH_BSLASH_QUOTE_END)), Pattern.compile(String.format(Locale.ROOT, "%s%s\\.%s%s", REGEX_CMD_PATH_BSLASH, basenameRegex, - extensionRegex, REGEX_CMD_PATH_BSLASH_END)).matcher("") }; + extensionRegex, REGEX_CMD_PATH_BSLASH_END)) }; } else { - this.toolNameMatchersExtBackslash = new Matcher[] {}; + this.toolNamePatternsExtBackslash = new Pattern[] {}; } } else { - this.toolNameMatchersExt = new Matcher[] {}; - this.toolNameMatchersExtBackslash = new Matcher[] {}; + this.toolNamePatternsExt = new Pattern[] {}; + this.toolNamePatternsExtBackslash = new Pattern[] {}; + } + + toolNameMatchersExt = new Matcher[toolNamePatternsExt.length]; + for (int i = 0; i < toolNameMatchersExt.length; i++) { + toolNameMatchersExt[i] = toolNamePatternsExt[i].matcher(""); + } + toolNameMatchersExtBackslash = new Matcher[toolNamePatternsExtBackslash.length]; + for (int i = 0; i < toolNameMatchersExtBackslash.length; i++) { + toolNameMatchersExtBackslash[i] = toolNamePatternsExtBackslash[i].matcher(("")); } } @@ -225,8 +248,8 @@ public class DefaultToolDetectionParticipant implements IToolDetectionParticipan if (matchBackslash && !canHandleNtfsPaths()) { return Optional.empty(); } - for (Matcher matcher : matchBackslash ? toolNameMatchersBackslash : toolNameMatchers) { - Optional result = matcherMatches(matcher, commandLine); + for (Pattern pattern : matchBackslash ? toolNamePatternsBackslash : toolNamePatterns) { + Optional result = patternMatches(pattern, commandLine); if (result.isPresent()) { return result; } @@ -242,23 +265,23 @@ public class DefaultToolDetectionParticipant implements IToolDetectionParticipan return Optional.empty(); } - Matcher[] toolNameMatchers; + Pattern[] toolNamePatterns; if (matchBackslash) { - toolNameMatchers = new Matcher[] { + toolNamePatterns = new Pattern[] { Pattern.compile(String.format(Locale.ROOT, "%s%s%s%s", REGEX_CMD_PATH_BSLASH_QUOTE, basenameRegex, - versionRegex, REGEX_CMD_PATH_BSLASH_QUOTE_END)).matcher(""), + versionRegex, REGEX_CMD_PATH_BSLASH_QUOTE_END)), Pattern.compile(String.format(Locale.ROOT, "%s%s%s%s", REGEX_CMD_PATH_BSLASH, basenameRegex, - versionRegex, REGEX_CMD_PATH_BSLASH_END)).matcher("") }; + versionRegex, REGEX_CMD_PATH_BSLASH_END)) }; } else { - toolNameMatchers = new Matcher[] { + toolNamePatterns = new Pattern[] { Pattern.compile(String.format(Locale.ROOT, "%s%s%s%s", REGEX_CMD_PATH_SLASH_QUOTE, basenameRegex, - versionRegex, REGEX_CMD_PATH_SLASH_QUOTE_END)).matcher(""), + versionRegex, REGEX_CMD_PATH_SLASH_QUOTE_END)), Pattern.compile(String.format(Locale.ROOT, "%s%s%s%s", REGEX_CMD_PATH_SLASH, basenameRegex, - versionRegex, REGEX_CMD_PATH_SLASH_END)).matcher("") }; + versionRegex, REGEX_CMD_PATH_SLASH_END)) }; } - for (Matcher matcher : toolNameMatchers) { - Optional result = matcherMatches(matcher, commandLine); + for (Pattern pattern : toolNamePatterns) { + Optional result = patternMatches(pattern, commandLine); if (result.isPresent()) { return result; } @@ -271,8 +294,8 @@ public class DefaultToolDetectionParticipant implements IToolDetectionParticipan if (matchBackslash && !canHandleNtfsPaths()) { return Optional.empty(); } - for (Matcher matcher : matchBackslash ? toolNameMatchersExtBackslash : toolNameMatchersExt) { - Optional result = matcherMatches(matcher, commandLine); + for (Pattern pattern : matchBackslash ? toolNamePatternsExtBackslash : toolNamePatternsExt) { + Optional result = patternMatches(pattern, commandLine); if (result.isPresent()) { return result; } @@ -288,23 +311,23 @@ public class DefaultToolDetectionParticipant implements IToolDetectionParticipan return Optional.empty(); } - Matcher[] toolNameMatchers; + Pattern[] toolNamePatterns; if (matchBackslash) { - toolNameMatchers = new Matcher[] { + toolNamePatterns = new Pattern[] { Pattern.compile(String.format(Locale.ROOT, "%s%s%s\\.%s%s", REGEX_CMD_PATH_BSLASH_QUOTE, - basenameRegex, versionRegex, extensionRegex, REGEX_CMD_PATH_BSLASH_QUOTE_END)).matcher(""), + basenameRegex, versionRegex, extensionRegex, REGEX_CMD_PATH_BSLASH_QUOTE_END)), Pattern.compile(String.format(Locale.ROOT, "%s%s%s\\.%s%s", REGEX_CMD_PATH_BSLASH, basenameRegex, - versionRegex, extensionRegex, REGEX_CMD_PATH_BSLASH_END)).matcher("") }; + versionRegex, extensionRegex, REGEX_CMD_PATH_BSLASH_END)) }; } else { - toolNameMatchers = new Matcher[] { + toolNamePatterns = new Pattern[] { Pattern.compile(String.format(Locale.ROOT, "%s%s%s\\.%s%s", REGEX_CMD_PATH_SLASH_QUOTE, - basenameRegex, versionRegex, extensionRegex, REGEX_CMD_PATH_SLASH_QUOTE_END)).matcher(""), + basenameRegex, versionRegex, extensionRegex, REGEX_CMD_PATH_SLASH_QUOTE_END)), Pattern.compile(String.format(Locale.ROOT, "%s%s%s\\.%s%s", REGEX_CMD_PATH_SLASH, basenameRegex, - versionRegex, extensionRegex, REGEX_CMD_PATH_SLASH_END)).matcher("") }; + versionRegex, extensionRegex, REGEX_CMD_PATH_SLASH_END)) }; } - for (Matcher matcher : toolNameMatchers) { - Optional result = matcherMatches(matcher, commandLine); + for (Pattern pattern : toolNamePatterns) { + Optional result = patternMatches(pattern, commandLine); if (result.isPresent()) { return result; } @@ -313,11 +336,11 @@ public class DefaultToolDetectionParticipant implements IToolDetectionParticipan } /** - * Gets, whether the specified Matcher for the tool arguments can properly parse + * Gets, whether the specified Pattern for the tool arguments can properly parse * the specified command-line string. If so, the remaining arguments of the * command-line are returned. * - * @param matcher the matcher that performs the match a regular expression + * @param pattern the pattern that performs the match a regular expression * that matches the version string in the name of the tool to * detect. * @param commandLine the command-line to match @@ -325,8 +348,8 @@ public class DefaultToolDetectionParticipant implements IToolDetectionParticipan * command-line string. Otherwise, if the tool name matches, a * MatchResult holding the de-composed command-line is returned. */ - private Optional matcherMatches(Matcher matcher, String commandLine) { - matcher.reset(commandLine); + private Optional patternMatches(Pattern pattern, String commandLine) { + Matcher matcher = pattern.matcher(commandLine); if (matcher.lookingAt()) { return Optional.of(new DefaultToolDetectionParticipant.MatchResult(matcher.group(REGEX_GROUP_CMD), commandLine.substring(matcher.end()))); diff --git a/jsoncdb/org.eclipse.cdt.jsoncdb.core/src/org/eclipse/cdt/jsoncdb/core/participant/IToolDetectionParticipant.java b/jsoncdb/org.eclipse.cdt.jsoncdb.core/src/org/eclipse/cdt/jsoncdb/core/participant/IToolDetectionParticipant.java index fbbf9b1b51b..0a2d782872f 100644 --- a/jsoncdb/org.eclipse.cdt.jsoncdb.core/src/org/eclipse/cdt/jsoncdb/core/participant/IToolDetectionParticipant.java +++ b/jsoncdb/org.eclipse.cdt.jsoncdb.core/src/org/eclipse/cdt/jsoncdb/core/participant/IToolDetectionParticipant.java @@ -9,6 +9,7 @@ package org.eclipse.cdt.jsoncdb.core.participant; +import java.util.Objects; import java.util.Optional; /** @@ -124,8 +125,8 @@ public interface IToolDetectionParticipant { * command */ public MatchResult(String command, String arguments) { - this.command = command; - this.arguments = arguments; + this.command = Objects.requireNonNull(command, "command"); //$NON-NLS-1$ + this.arguments = Objects.requireNonNull(arguments, "arguments"); //$NON-NLS-1$ } /** diff --git a/jsoncdb/org.eclipse.cdt.jsoncdb.core/src/org/eclipse/cdt/jsoncdb/core/participant/ResponseFileArglets.java b/jsoncdb/org.eclipse.cdt.jsoncdb.core/src/org/eclipse/cdt/jsoncdb/core/participant/ResponseFileArglets.java index 6f6f6469083..16f162eb2af 100644 --- a/jsoncdb/org.eclipse.cdt.jsoncdb.core/src/org/eclipse/cdt/jsoncdb/core/participant/ResponseFileArglets.java +++ b/jsoncdb/org.eclipse.cdt.jsoncdb.core/src/org/eclipse/cdt/jsoncdb/core/participant/ResponseFileArglets.java @@ -55,9 +55,8 @@ public class ResponseFileArglets { @Override public int process(IParserHandler parserHandler, String argsLine) { for (NameOptionMatcher oMatcher : optionMatchers) { - final Matcher matcher = oMatcher.matcher; + final Matcher matcher = oMatcher.pattern.matcher(argsLine); - matcher.reset(argsLine); if (matcher.lookingAt()) { String fname = matcher.group(oMatcher.nameGroup); final int consumed = matcher.end();