Skip to content

Commit

Permalink
ProjectVariableProviderManager: fix concurrency issues
Browse files Browse the repository at this point in the history 
lazyInitialize used wrong double-checked locking idiom
  • Loading branch information
@EcljpseB0T @jukzi
EcljpseB0T authored and jukzi committed Mar 6, 2024
1 parent 6718133 commit d078b2c
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 30 deletions.
14 changes: 3 additions & 11 deletions ...se.core.resources/src/org/eclipse/core/internal/resources/ProjectPathVariableManager.java
View file
Expand Up @@ -20,7 +20,6 @@
import java.net.URISyntaxException;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import org.eclipse.core.filesystem.URIUtil;
Expand Down Expand Up @@ -48,7 +47,7 @@
public class ProjectPathVariableManager implements IPathVariableManager, IManager {

private final Resource resource;
private ProjectVariableProviderManager.Descriptor variableProviders[] = null;
private final ProjectVariableProviderManager.Descriptor variableProviders[];

/**
* Constructor for the class.
Expand Down Expand Up @@ -182,19 +181,14 @@ public String internalGetValue(String varName) {
@Override
public boolean isDefined(String varName) {
for (Descriptor variableProvider : variableProviders) {
// if (variableProviders[i].getName().equals(varName))
// return true;

if (varName.startsWith(variableProvider.getName()))
return true;
}

try {
HashMap<String, VariableDescription> map = ((ProjectDescription) resource.getProject().getDescription()).getVariables();
if (map != null) {
Iterator<String> it = map.keySet().iterator();
while (it.hasNext()) {
String name = it.next();
for (String name : map.keySet()) {
if (name.equals(varName))
return true;
}
Expand Down Expand Up @@ -358,9 +352,7 @@ public void setURIValue(String varName, URI newValue) throws CoreException {
}
}
boolean variableExists = currentValue != null;
if (!variableExists && newValue == null)
return;
if (variableExists && currentValue.equals(newValue))
if ((!variableExists && newValue == null) || (variableExists && currentValue.equals(newValue)))
return;

for (Descriptor variableProvider : variableProviders) {
Expand Down
40 changes: 21 additions & 19 deletions ...ore.resources/src/org/eclipse/core/internal/resources/ProjectVariableProviderManager.java
View file
Expand Up @@ -23,7 +23,13 @@
import org.eclipse.core.resources.IResource;
import org.eclipse.core.resources.ResourcesPlugin;
import org.eclipse.core.resources.variableresolvers.PathVariableResolver;
import org.eclipse.core.runtime.*;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IConfigurationElement;
import org.eclipse.core.runtime.IExtension;
import org.eclipse.core.runtime.IExtensionPoint;
import org.eclipse.core.runtime.IStatus;
import org.eclipse.core.runtime.Platform;
import org.eclipse.core.runtime.Status;
import org.eclipse.osgi.util.NLS;

/**
Expand All @@ -33,20 +39,22 @@
public class ProjectVariableProviderManager {

public static class Descriptor {
PathVariableResolver provider = null;
String name = null;
String value = null;
private final PathVariableResolver provider;
private final String name;
private final String value;

public Descriptor(IExtension extension, IConfigurationElement element) throws RuntimeException, CoreException {
name = element.getAttribute("variable"); //$NON-NLS-1$
value = element.getAttribute("value"); //$NON-NLS-1$
PathVariableResolver p = null;
try {
String classAttribute = "class"; //$NON-NLS-1$
if (element.getAttribute(classAttribute) != null)
provider = (PathVariableResolver) element.createExecutableExtension(classAttribute);
p = (PathVariableResolver) element.createExecutableExtension(classAttribute);
} catch (CoreException e) {
Policy.log(e);
}
provider = p;
if (name == null)
fail(NLS.bind(Messages.mapping_invalidDef, extension.getUniqueIdentifier()));
}
Expand Down Expand Up @@ -74,30 +82,25 @@ public String[] getVariableNames(String variable, IResource resource) {
}
}

private static Map<String, Descriptor> descriptors;
private static Descriptor[] descriptorsArray;
private static ProjectVariableProviderManager instance = new ProjectVariableProviderManager();
private static final Map<String, Descriptor> descriptors = getDescriptorMap();
private static final Descriptor[] descriptorsArray = descriptors.values().toArray(Descriptor[]::new);
private static final ProjectVariableProviderManager instance = new ProjectVariableProviderManager();

public static ProjectVariableProviderManager getDefault() {
return instance;
}

public Descriptor[] getDescriptors() {
lazyInitialize();
return descriptorsArray;
}

protected void lazyInitialize() {
if (descriptors != null)
return;
private static Map<String, Descriptor> getDescriptorMap() {
IExtensionPoint point = Platform.getExtensionRegistry().getExtensionPoint(ResourcesPlugin.PI_RESOURCES, ResourcesPlugin.PT_VARIABLE_PROVIDERS);
IExtension[] extensions = point.getExtensions();
descriptors = new HashMap<>(extensions.length * 2 + 1);
Map<String, Descriptor> d = new HashMap<>(extensions.length * 2 + 1);
for (IExtension extension : extensions) {
IConfigurationElement[] elements = extension.getConfigurationElements();
int count = elements.length;
for (int j = 0; j < count; j++) {
IConfigurationElement element = elements[j];
for (IConfigurationElement element : elements) {
String elementName = element.getName();
if (elementName.equalsIgnoreCase("variableResolver")) { //$NON-NLS-1$
Descriptor desc = null;
Expand All @@ -107,15 +110,14 @@ protected void lazyInitialize() {
Policy.log(e);
}
if (desc != null)
descriptors.put(desc.getName(), desc);
d.put(desc.getName(), desc);
}
}
}
descriptorsArray = descriptors.values().toArray(new Descriptor[descriptors.size()]);
return Map.copyOf(d);
}

public Descriptor findDescriptor(String name) {
lazyInitialize();
Descriptor result = descriptors.get(name);
return result;
}
Expand Down

0 comments on commit d078b2c

Please sign in to comment.