Skip to content

Commit

Permalink
Fix potential problems with concurrent access to hash maps
Browse files Browse the repository at this point in the history 
Signed-off-by: Ivan Kochurkin <[email protected]>
  • Loading branch information
@KvanTTT @parrt
KvanTTT authored and parrt committed Jun 26, 2022
1 parent a7c558b commit 7e8c4f4
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 84 deletions.
14 changes: 4 additions & 10 deletions runtime-testsuite/test/org/antlr/v4/test/runtime/RuntimeTestUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ public abstract class RuntimeTestUtils {
public final static Path runtimeTestsuitePath;
public final static Path resourcePath;

private final static Object resourceLockObject = new Object();
private final static Map<String, String> resourceCache = new HashMap<>();
private static OSType detectedOS;
private static Boolean isWindows;
Expand Down Expand Up @@ -81,18 +80,13 @@ else if (os.contains("nux")) {
return detectedOS;
}

public static String getTextFromResource(String name) {
public static synchronized String getTextFromResource(String name) {
try {
String text = resourceCache.get(name);
if (text == null) {
synchronized (resourceLockObject) {
text = resourceCache.get(name);
if (text == null) {
Path path = Paths.get(resourcePath.toString(), name);
text = new String(Files.readAllBytes(path));
resourceCache.put(name, text);
}
}
Path path = Paths.get(resourcePath.toString(), name);
text = new String(Files.readAllBytes(path));
resourceCache.put(name, text);
}
return text;
}
Expand Down
32 changes: 14 additions & 18 deletions runtime-testsuite/test/org/antlr/v4/test/runtime/RuntimeTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,7 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.*;
import java.util.stream.Stream;

import static org.antlr.v4.test.runtime.FileUtils.writeFile;
Expand All @@ -45,7 +42,7 @@ public abstract class RuntimeTests {
protected abstract RuntimeRunner createRuntimeRunner();

private final static HashMap<String, RuntimeTestDescriptor[]> testDescriptors = new HashMap<>();
private final static HashMap<String, STGroup> cachedTargetTemplates = new HashMap<>();
private final static Map<String, STGroup> cachedTargetTemplates = new HashMap<>();
private final static StringRenderer rendered = new StringRenderer();

static {
Expand Down Expand Up @@ -179,24 +176,23 @@ private static String test(RuntimeTestDescriptor descriptor, RuntimeRunner runne

private static String prepareGrammars(RuntimeTestDescriptor descriptor, RuntimeRunner runner) {
String targetName = runner.getLanguage();
STGroup targetTemplates = cachedTargetTemplates.get(targetName);
if (targetTemplates == null) {
synchronized (cachedTargetTemplates) {
targetTemplates = cachedTargetTemplates.get(targetName);
if (targetTemplates == null) {
ClassLoader classLoader = RuntimeTests.class.getClassLoader();
URL templates = classLoader.getResource("org/antlr/v4/test/runtime/templates/" + targetName + ".test.stg");
assert templates != null;
targetTemplates = new STGroupFile(templates, "UTF-8", '<', '>');
targetTemplates.registerRenderer(String.class, rendered);
cachedTargetTemplates.put(targetName, targetTemplates);
}

STGroup targetTemplates;
synchronized (cachedTargetTemplates) {
targetTemplates = cachedTargetTemplates.get(targetName);
if (targetTemplates == null) {
ClassLoader classLoader = RuntimeTests.class.getClassLoader();
URL templates = classLoader.getResource("org/antlr/v4/test/runtime/templates/" + targetName + ".test.stg");
assert templates != null;
targetTemplates = new STGroupFile(templates, "UTF-8", '<', '>');
targetTemplates.registerRenderer(String.class, rendered);
cachedTargetTemplates.put(targetName, targetTemplates);
}
}

// write out any slave grammars
List<Pair<String, String>> slaveGrammars = descriptor.slaveGrammars;
if ( slaveGrammars!=null ) {
if (slaveGrammars != null) {
for (Pair<String, String> spair : slaveGrammars) {
STGroup g = new STGroup('<', '>');
g.registerRenderer(String.class, rendered);
Expand Down
19 changes: 7 additions & 12 deletions tool/src/org/antlr/v4/codegen/Target.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,23 +87,18 @@ public String getVersion() {
return Tool.VERSION;
}

public STGroup getTemplates() {
public synchronized STGroup getTemplates() {
String language = getLanguage();
STGroup templates = languageTemplates.get(language);

if (templates == null) {
synchronized (languageTemplates) {
templates = languageTemplates.get(language);
if (templates == null) {
String version = getVersion();
if (version == null ||
!RuntimeMetaData.getMajorMinorVersion(version).equals(RuntimeMetaData.getMajorMinorVersion(Tool.VERSION))) {
gen.tool.errMgr.toolError(ErrorType.INCOMPATIBLE_TOOL_AND_TEMPLATES, version, Tool.VERSION, language);
}
templates = loadTemplates();
languageTemplates.put(language, templates);
}
String version = getVersion();
if (version == null ||
!RuntimeMetaData.getMajorMinorVersion(version).equals(RuntimeMetaData.getMajorMinorVersion(Tool.VERSION))) {
gen.tool.errMgr.toolError(ErrorType.INCOMPATIBLE_TOOL_AND_TEMPLATES, version, Tool.VERSION, language);
}
templates = loadTemplates();
languageTemplates.put(language, templates);
}

return templates;
Expand Down
84 changes: 40 additions & 44 deletions tool/src/org/antlr/v4/tool/ErrorManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,10 @@

import java.io.File;
import java.net.URL;
import java.util.Collection;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.Set;
import java.util.*;

public class ErrorManager {
private final static HashMap<String, STGroupFile> loadedFormats = new HashMap<>();
private final static Map<String, STGroupFile> loadedFormats = new HashMap<>();

public static final String FORMATS_DIR = "org/antlr/v4/tool/templates/messages/formats/";

Expand Down Expand Up @@ -219,53 +216,52 @@ public void emit(ErrorType etype, ANTLRMessage msg) {
* Otherwise we just use the default "antlr".
*/
public void setFormat(String formatName) {
STGroupFile loadedFormat = loadedFormats.get(formatName);
if (loadedFormat == null) {
synchronized (loadedFormats) {
loadedFormat = loadedFormats.get(formatName);
if (loadedFormat == null) {
String fileName = FORMATS_DIR +formatName+STGroup.GROUP_FILE_EXTENSION;
ClassLoader cl = Thread.currentThread().getContextClassLoader();
URL url = cl.getResource(fileName);
if ( url==null ) {
cl = ErrorManager.class.getClassLoader();
url = cl.getResource(fileName);
}
if ( url==null && formatName.equals("antlr") ) {
rawError("ANTLR installation corrupted; cannot find ANTLR messages format file "+fileName);
panic();
}
else if ( url==null ) {
rawError("no such message format file "+fileName+" retrying with default ANTLR format");
setFormat("antlr"); // recurse on this rule, trying the default message format
return;
}
loadedFormat = new STGroupFile(url, "UTF-8", '<', '>');
loadedFormat.load();

loadedFormats.put(formatName, loadedFormat);
STGroupFile loadedFormat;

synchronized (loadedFormats) {
loadedFormat = loadedFormats.get(formatName);
if (loadedFormat == null) {
String fileName = FORMATS_DIR + formatName + STGroup.GROUP_FILE_EXTENSION;
ClassLoader cl = Thread.currentThread().getContextClassLoader();
URL url = cl.getResource(fileName);
if (url == null) {
cl = ErrorManager.class.getClassLoader();
url = cl.getResource(fileName);
}
if (url == null && formatName.equals("antlr")) {
rawError("ANTLR installation corrupted; cannot find ANTLR messages format file " + fileName);
panic();
}
else if (url == null) {
rawError("no such message format file " + fileName + " retrying with default ANTLR format");
setFormat("antlr"); // recurse on this rule, trying the default message format
return;
}
loadedFormat = new STGroupFile(url, "UTF-8", '<', '>');
loadedFormat.load();

loadedFormats.put(formatName, loadedFormat);
}
}

this.formatName = formatName;
this.format = loadedFormat;

if ( !initSTListener.errors.isEmpty() ) {
rawError("ANTLR installation corrupted; can't load messages format file:\n"+
initSTListener.toString());
panic();
}
if (!initSTListener.errors.isEmpty()) {
rawError("ANTLR installation corrupted; can't load messages format file:\n" +
initSTListener.toString());
panic();
}

boolean formatOK = verifyFormat();
if ( !formatOK && formatName.equals("antlr") ) {
rawError("ANTLR installation corrupted; ANTLR messages format file "+formatName+".stg incomplete");
panic();
}
else if ( !formatOK ) {
setFormat("antlr"); // recurse on this rule, trying the default message format
}
}
boolean formatOK = verifyFormat();
if (!formatOK && formatName.equals("antlr")) {
rawError("ANTLR installation corrupted; ANTLR messages format file " + formatName + ".stg incomplete");
panic();
}
else if (!formatOK) {
setFormat("antlr"); // recurse on this rule, trying the default message format
}
}

/** Verify the message format template group */
protected boolean verifyFormat() {
Expand Down

0 comments on commit 7e8c4f4

Please sign in to comment.