Bug 578683: o.e.cdt.jsoncdb.core Arglets are not thread safe

Change-Id: I5f54d6729cffcf47382a37a436a2772a5ca84340
Signed-off-by: Martin Weber <fifteenknots505@gmail.com>

doc/org.eclipse.cdt.doc.isv/guide/deprecated_API_removals.html

@@ -70,6 +70,12 @@
     <li><a href="#GnuMakefileGeneratorAPI">GnuMakefileGenerator is no longer part of API</a></li>
     <li><a href="#Spawner.signals">The Spawner signal constants are nolonger API</a></li>
   </ol>
+  <p>
+    Planned Removals after March 2024
+  </p>
+  <ol>
+    <li><a href="#ArgletsMatcher">java.util.regex.Matcher use in JSONCDB API will be removed</a></li>
+  </ol>
 
   <p>
     API Changes prior to CDT 10.0 / 2020-09.
@@ -524,6 +530,23 @@
     <li>INT</li>
     <li>CTRLC</li>
   </ul>
+
+
+  <h3>API Removals after March 2024</h3>
+
+  <h3>1. <a name="ArgletsMatcher">java.util.regex.Matcher use in JSONCDB API will be removed</a></h3>
+  <p>
+    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.
+  </p>
+  <ul>
+    <li>org.eclipse.cdt.jsoncdb.core.participant.Arglets.NameOptionMatcher.matcher</li>
+    <li>org.eclipse.cdt.jsoncdb.core.participant.DefaultToolDetectionParticipant.toolNameMatchersExt</li>
+    <li>org.eclipse.cdt.jsoncdb.core.participant.DefaultToolDetectionParticipant.toolNameMatchersExtBackslash</li>
+  </ul>
+  <p>
+    See <a href="https://bugs.eclipse.org/bugs/show_bug.cgi?id=578683" target="_blank">Bug 578683</a>.
+  </p>
+
 </body>
 
 </html>

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.detectorWMethod = Objects.requireNonNull(detectorWMethod, "detectorWMethod");
-			this.commandLine = commandLine;
+			this.commandLine = Objects.requireNonNull(commandLine, "commandLine");
 		}
 
 		/**

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 pattern  regular expression pattern being parsed by the parser.
-		 * @param nameGroup  - capturing group number defining name of an entry.
+		 * @param nameGroup the number of the value group
-		 * @param valueGroup - capturing group number defining value of an entry.
-		 */
-		/**
-		 * @param pattern
-		 * @param nameGroup
 		 * @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);

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 Pattern[] toolNamePatterns;
-	private final Matcher[] toolNameMatchersBackslash;
+	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.toolNamePatternsExt = new Pattern[] {};
-			this.toolNameMatchersExtBackslash = new Matcher[] {};
+			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) {
+		for (Pattern pattern : matchBackslash ? toolNamePatternsBackslash : toolNamePatterns) {
-			Optional<MatchResult> result = matcherMatches(matcher, commandLine);
+			Optional<MatchResult> 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) {
+		for (Pattern pattern : toolNamePatterns) {
-			Optional<MatchResult> result = matcherMatches(matcher, commandLine);
+			Optional<MatchResult> 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) {
+		for (Pattern pattern : matchBackslash ? toolNamePatternsExtBackslash : toolNamePatternsExt) {
-			Optional<MatchResult> result = matcherMatches(matcher, commandLine);
+			Optional<MatchResult> 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) {
+		for (Pattern pattern : toolNamePatterns) {
-			Optional<MatchResult> result = matcherMatches(matcher, commandLine);
+			Optional<MatchResult> 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<DefaultToolDetectionParticipant.MatchResult> matcherMatches(Matcher matcher, String commandLine) {
+	private Optional<DefaultToolDetectionParticipant.MatchResult> patternMatches(Pattern pattern, String commandLine) {
-		matcher.reset(commandLine);
+		Matcher matcher = pattern.matcher(commandLine);
 		if (matcher.lookingAt()) {
 			return Optional.of(new DefaultToolDetectionParticipant.MatchResult(matcher.group(REGEX_GROUP_CMD),
 					commandLine.substring(matcher.end())));

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.command = Objects.requireNonNull(command, "command"); //$NON-NLS-1$
-			this.arguments = arguments;
+			this.arguments = Objects.requireNonNull(arguments, "arguments"); //$NON-NLS-1$
 		}
 
 		/**

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();