honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Painless: Fix Bug with Duplicate PainlessClasses (#32110) 

When building the PainlessMethods and PainlessFields they stored a reference to a 
PainlessClass. This reference was prior to "freezing" the PainlessClass so the data was 
both incomplete and mutable. This has been replaced with a target java class instead 
since the PainlessClass is accessible through a java class now and it requires no special 
modifications to get around a chicken and egg issue.
This commit is contained in:
Jack Conradson 2018-07-17 10:33:38 -07:00 committed by GitHub
parent 9cdbd1bd85
commit 1c63eb1081
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 62 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java
View File

@ -334,8 +334,8 @@ public final class Def {
}
int arity = interfaceMethod.arguments.size();
PainlessMethod implMethod = lookupMethodInternal(painlessLookup, receiverClass, name, arity);
return lookupReferenceInternal(painlessLookup, methodHandlesLookup, interfaceType, implMethod.owner.name,
implMethod.name, receiverClass);
return lookupReferenceInternal(painlessLookup, methodHandlesLookup, interfaceType,
PainlessLookupUtility.anyTypeToPainlessTypeName(implMethod.target), implMethod.name, receiverClass);
}
/** Returns a method handle to an implementation of clazz, given method reference signature. */

8
modules/lang-painless/src/main/java/org/elasticsearch/painless/FunctionRef.java
View File

@ -102,22 +102,22 @@ public class FunctionRef {
interfaceMethodType = interfaceMethod.getMethodType().dropParameterTypes(0, 1);
// the Painless$Script class can be inferred if owner is null
if (delegateMethod.owner == null) {
if (delegateMethod.target == null) {
delegateClassName = CLASS_NAME;
isDelegateInterface = false;
} else if (delegateMethod.augmentation != null) {
delegateClassName = delegateMethod.augmentation.getName();
isDelegateInterface = delegateMethod.augmentation.isInterface();
} else {
delegateClassName = delegateMethod.owner.clazz.getName();
isDelegateInterface = delegateMethod.owner.clazz.isInterface();
delegateClassName = delegateMethod.target.getName();
isDelegateInterface = delegateMethod.target.isInterface();
}
if ("<init>".equals(delegateMethod.name)) {
delegateInvokeType = H_NEWINVOKESPECIAL;
} else if (Modifier.isStatic(delegateMethod.modifiers)) {
delegateInvokeType = H_INVOKESTATIC;
} else if (delegateMethod.owner.clazz.isInterface()) {
} else if (delegateMethod.target.isInterface()) {
delegateInvokeType = H_INVOKEINTERFACE;
} else {
delegateInvokeType = H_INVOKEVIRTUAL;

6
modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessField.java
View File

@ -23,18 +23,18 @@ import java.lang.invoke.MethodHandle;
public final class PainlessField {
public final String name;
public final PainlessClass owner;
public final Class<?> target;
public final Class<?> clazz;
public final String javaName;
public final int modifiers;
public final MethodHandle getter;
public final MethodHandle setter;
PainlessField(String name, String javaName, PainlessClass owner, Class<?> clazz, int modifiers,
PainlessField(String name, String javaName, Class<?> target, Class<?> clazz, int modifiers,
MethodHandle getter, MethodHandle setter) {
this.name = name;
this.javaName = javaName;
this.owner = owner;
this.target = target;
this.clazz = clazz;
this.modifiers = modifiers;
this.getter = getter;

14
modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java
View File

@ -310,7 +310,7 @@ public class PainlessLookupBuilder {
}
painlessConstructor = methodCache.computeIfAbsent(buildMethodCacheKey(ownerStruct.name, "<init>", painlessParametersTypes),
key -> new PainlessMethod("<init>", ownerStruct, null, void.class, painlessParametersTypes,
key -> new PainlessMethod("<init>", ownerStruct.clazz, null, void.class, painlessParametersTypes,
asmConstructor, javaConstructor.getModifiers(), javaHandle));
ownerStruct.constructors.put(painlessMethodKey, painlessConstructor);
} else if (painlessConstructor.arguments.equals(painlessParametersTypes) == false){
@ -419,7 +419,7 @@ public class PainlessLookupBuilder {
painlessMethod = methodCache.computeIfAbsent(
buildMethodCacheKey(ownerStruct.name, whitelistMethod.javaMethodName, painlessParametersTypes),
key -> new PainlessMethod(whitelistMethod.javaMethodName, ownerStruct, null, painlessReturnClass,
key -> new PainlessMethod(whitelistMethod.javaMethodName, ownerStruct.clazz, null, painlessReturnClass,
painlessParametersTypes, asmMethod, javaMethod.getModifiers(), javaMethodHandle));
ownerStruct.staticMethods.put(painlessMethodKey, painlessMethod);
} else if ((painlessMethod.name.equals(whitelistMethod.javaMethodName) && painlessMethod.rtn == painlessReturnClass &&
@ -445,7 +445,7 @@ public class PainlessLookupBuilder {
painlessMethod = methodCache.computeIfAbsent(
buildMethodCacheKey(ownerStruct.name, whitelistMethod.javaMethodName, painlessParametersTypes),
key -> new PainlessMethod(whitelistMethod.javaMethodName, ownerStruct, javaAugmentedClass, painlessReturnClass,
key -> new PainlessMethod(whitelistMethod.javaMethodName, ownerStruct.clazz, javaAugmentedClass, painlessReturnClass,
painlessParametersTypes, asmMethod, javaMethod.getModifiers(), javaMethodHandle));
ownerStruct.methods.put(painlessMethodKey, painlessMethod);
} else if ((painlessMethod.name.equals(whitelistMethod.javaMethodName) && painlessMethod.rtn.equals(painlessReturnClass) &&
@ -501,7 +501,7 @@ public class PainlessLookupBuilder {
painlessField = fieldCache.computeIfAbsent(
buildFieldCacheKey(ownerStruct.name, whitelistField.javaFieldName, painlessFieldClass.getName()),
key -> new PainlessField(whitelistField.javaFieldName, javaField.getName(),
ownerStruct, painlessFieldClass, javaField.getModifiers(), null, null));
ownerStruct.clazz, painlessFieldClass, javaField.getModifiers(), null, null));
ownerStruct.staticMembers.put(whitelistField.javaFieldName, painlessField);
} else if (painlessField.clazz != painlessFieldClass) {
throw new IllegalArgumentException("illegal duplicate static fields [" + whitelistField.javaFieldName + "] " +
@ -530,7 +530,7 @@ public class PainlessLookupBuilder {
painlessField = fieldCache.computeIfAbsent(
buildFieldCacheKey(ownerStruct.name, whitelistField.javaFieldName, painlessFieldClass.getName()),
key -> new PainlessField(whitelistField.javaFieldName, javaField.getName(),
ownerStruct, painlessFieldClass, javaField.getModifiers(), javaMethodHandleGetter, javaMethodHandleSetter));
ownerStruct.clazz, painlessFieldClass, javaField.getModifiers(), javaMethodHandleGetter, javaMethodHandleSetter));
ownerStruct.members.put(whitelistField.javaFieldName, painlessField);
} else if (painlessField.clazz != painlessFieldClass) {
throw new IllegalArgumentException("illegal duplicate member fields [" + whitelistField.javaFieldName + "] " +
@ -615,8 +615,8 @@ public class PainlessLookupBuilder {
for (PainlessField field : child.members.values()) {
if (owner.members.get(field.name) == null) {
owner.members.put(field.name,
new PainlessField(field.name, field.javaName, owner, field.clazz, field.modifiers, field.getter, field.setter));
owner.members.put(field.name, new PainlessField(
field.name, field.javaName, owner.clazz, field.clazz, field.modifiers, field.getter, field.setter));
}
}
}

6
modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupUtility.java
View File

@ -158,7 +158,7 @@ public final class PainlessLookupUtility {
painlessTypeName.charAt(arrayIndex++) == ']') {
++arrayDimensions;
} else {
throw new IllegalArgumentException("invalid painless type [" + painlessTypeName + "].");
throw new IllegalArgumentException("painless type [" + painlessTypeName + "] not found");
}
}
@ -192,7 +192,7 @@ public final class PainlessLookupUtility {
try {
return Class.forName(javaDescriptor);
} catch (ClassNotFoundException cnfe) {
throw new IllegalStateException("painless type [" + painlessTypeName + "] not found", cnfe);
throw new IllegalArgumentException("painless type [" + painlessTypeName + "] not found", cnfe);
}
}
@ -207,7 +207,7 @@ public final class PainlessLookupUtility {
}
if (javaClasses.contains(painlessType) == false) {
throw new IllegalStateException("painless type [" + painlessTypeName + "] not found");
throw new IllegalArgumentException("painless type [" + painlessTypeName + "] not found");
}
}

15
modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessMethod.java
View File

@ -21,6 +21,7 @@ package org.elasticsearch.painless.lookup;
import org.elasticsearch.painless.MethodWriter;
import org.objectweb.asm.Opcodes;
import org.objectweb.asm.Type;
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodType;
@ -30,7 +31,7 @@ import java.util.List;
public class PainlessMethod {
public final String name;
public final PainlessClass owner;
public final Class<?> target;
public final Class<?> augmentation;
public final Class<?> rtn;
public final List<Class<?>> arguments;
@ -38,11 +39,11 @@ public class PainlessMethod {
public final int modifiers;
public final MethodHandle handle;
public PainlessMethod(String name, PainlessClass owner, Class<?> augmentation, Class<?> rtn, List<Class<?>> arguments,
public PainlessMethod(String name, Class<?> target, Class<?> augmentation, Class<?> rtn, List<Class<?>> arguments,
org.objectweb.asm.commons.Method method, int modifiers, MethodHandle handle) {
this.name = name;
this.augmentation = augmentation;
this.owner = owner;
this.target = target;
this.rtn = rtn;
this.arguments = Collections.unmodifiableList(arguments);
this.method = method;
@ -85,11 +86,11 @@ public class PainlessMethod {
for (int i = 0; i < arguments.size(); i++) {
params[i] = PainlessLookupUtility.painlessDefTypeToJavaObjectType(arguments.get(i));
}
returnValue = owner.clazz;
returnValue = target;
} else {
// virtual/interface method: add receiver class
params = new Class<?>[1 + arguments.size()];
params[0] = owner.clazz;
params[0] = target;
for (int i = 0; i < arguments.size(); i++) {
params[i + 1] = PainlessLookupUtility.painlessDefTypeToJavaObjectType(arguments.get(i));
}
@ -106,8 +107,8 @@ public class PainlessMethod {
clazz = augmentation;
type = org.objectweb.asm.Type.getType(augmentation);
} else {
clazz = owner.clazz;
type = owner.type;
clazz = target;
type = Type.getType(target);
}
if (Modifier.isStatic(modifiers)) {

3
modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EListInit.java
View File

@ -26,6 +26,7 @@ import org.elasticsearch.painless.MethodWriter;
import org.elasticsearch.painless.lookup.PainlessMethod;
import org.elasticsearch.painless.lookup.PainlessMethodKey;
import org.elasticsearch.painless.lookup.def;
import org.objectweb.asm.Type;
import java.util.ArrayList;
import java.util.List;
@ -90,7 +91,7 @@ public final class EListInit extends AExpression {
writer.newInstance(MethodWriter.getType(actual));
writer.dup();
writer.invokeConstructor(constructor.owner.type, constructor.method);
writer.invokeConstructor(Type.getType(constructor.target), constructor.method);
for (AExpression value : values) {
writer.dup();

3
modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EMapInit.java
View File

@ -26,6 +26,7 @@ import org.elasticsearch.painless.MethodWriter;
import org.elasticsearch.painless.lookup.PainlessMethod;
import org.elasticsearch.painless.lookup.PainlessMethodKey;
import org.elasticsearch.painless.lookup.def;
import org.objectweb.asm.Type;
import java.util.HashMap;
import java.util.List;
@ -109,7 +110,7 @@ public final class EMapInit extends AExpression {
writer.newInstance(MethodWriter.getType(actual));
writer.dup();
writer.invokeConstructor(constructor.owner.type, constructor.method);
writer.invokeConstructor(Type.getType(constructor.target), constructor.method);
for (int index = 0; index < keys.size(); ++index) {
AExpression key = keys.get(index);

3
modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENewObj.java
View File

@ -26,6 +26,7 @@ import org.elasticsearch.painless.MethodWriter;
import org.elasticsearch.painless.lookup.PainlessClass;
import org.elasticsearch.painless.lookup.PainlessMethod;
import org.elasticsearch.painless.lookup.PainlessMethodKey;
import org.objectweb.asm.Type;
import java.util.List;
import java.util.Objects;
@ -104,7 +105,7 @@ public final class ENewObj extends AExpression {
argument.write(writer, globals);
}
writer.invokeConstructor(constructor.owner.type, constructor.method);
writer.invokeConstructor(Type.getType(constructor.target), constructor.method);
}
@Override

13
modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubField.java
View File

@ -25,6 +25,7 @@ import org.elasticsearch.painless.Location;
import org.elasticsearch.painless.MethodWriter;
import org.elasticsearch.painless.lookup.PainlessField;
import org.elasticsearch.painless.lookup.PainlessLookupUtility;
import org.objectweb.asm.Type;
import java.lang.reflect.Modifier;
import java.util.Objects;
@ -63,9 +64,9 @@ final class PSubField extends AStoreable {
writer.writeDebugInfo(location);
if (java.lang.reflect.Modifier.isStatic(field.modifiers)) {
writer.getStatic(field.owner.type, field.javaName, MethodWriter.getType(field.clazz));
writer.getStatic(Type.getType(field.target), field.javaName, MethodWriter.getType(field.clazz));
} else {
writer.getField(field.owner.type, field.javaName, MethodWriter.getType(field.clazz));
writer.getField(Type.getType(field.target), field.javaName, MethodWriter.getType(field.clazz));
}
}
@ -94,9 +95,9 @@ final class PSubField extends AStoreable {
writer.writeDebugInfo(location);
if (java.lang.reflect.Modifier.isStatic(field.modifiers)) {
writer.getStatic(field.owner.type, field.javaName, MethodWriter.getType(field.clazz));
writer.getStatic(Type.getType(field.target), field.javaName, MethodWriter.getType(field.clazz));
} else {
writer.getField(field.owner.type, field.javaName, MethodWriter.getType(field.clazz));
writer.getField(Type.getType(field.target), field.javaName, MethodWriter.getType(field.clazz));
}
}
@ -105,9 +106,9 @@ final class PSubField extends AStoreable {
writer.writeDebugInfo(location);
if (java.lang.reflect.Modifier.isStatic(field.modifiers)) {
writer.putStatic(field.owner.type, field.javaName, MethodWriter.getType(field.clazz));
writer.putStatic(Type.getType(field.target), field.javaName, MethodWriter.getType(field.clazz));
} else {
writer.putField(field.owner.type, field.javaName, MethodWriter.getType(field.clazz));
writer.putField(Type.getType(field.target), field.javaName, MethodWriter.getType(field.clazz));
}
}

42
modules/lang-painless/src/test/java/org/elasticsearch/painless/PainlessDocGenerator.java
View File

@ -27,6 +27,7 @@ import org.elasticsearch.painless.lookup.PainlessClass;
import org.elasticsearch.painless.lookup.PainlessField;
import org.elasticsearch.painless.lookup.PainlessLookup;
import org.elasticsearch.painless.lookup.PainlessLookupBuilder;
import org.elasticsearch.painless.lookup.PainlessLookupUtility;
import org.elasticsearch.painless.lookup.PainlessMethod;
import org.elasticsearch.painless.spi.Whitelist;
@ -67,8 +68,8 @@ public class PainlessDocGenerator {
Path indexPath = apiRootPath.resolve("index.asciidoc");
logger.info("Starting to write [index.asciidoc]");
try (PrintStream indexStream = new PrintStream(
Files.newOutputStream(indexPath, StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE),
false, StandardCharsets.UTF_8.name())) {
Files.newOutputStream(indexPath, StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE),
false, StandardCharsets.UTF_8.name())) {
emitGeneratedWarning(indexStream);
List<PainlessClass> structs = PAINLESS_LOOKUP.getStructs().stream().sorted(comparing(t -> t.name)).collect(toList());
for (PainlessClass struct : structs) {
@ -91,7 +92,7 @@ public class PainlessDocGenerator {
false, StandardCharsets.UTF_8.name())) {
emitGeneratedWarning(typeStream);
typeStream.print("[[");
emitAnchor(typeStream, struct);
emitAnchor(typeStream, struct.clazz);
typeStream.print("]]++");
typeStream.print(struct.name);
typeStream.println("++::");
@ -104,10 +105,11 @@ public class PainlessDocGenerator {
struct.constructors.values().stream().sorted(NUMBER_OF_ARGS).forEach(documentMethod);
Map<String, PainlessClass> inherited = new TreeMap<>();
struct.methods.values().stream().sorted(METHOD_NAME.thenComparing(NUMBER_OF_ARGS)).forEach(method -> {
if (method.owner == struct) {
if (method.target == struct.clazz) {
documentMethod(typeStream, method);
} else {
inherited.put(method.owner.name, method.owner);
PainlessClass painlessClass = PAINLESS_LOOKUP.getPainlessStructFromJavaClass(method.target);
inherited.put(painlessClass.name, painlessClass);
}
});
@ -206,16 +208,16 @@ public class PainlessDocGenerator {
/**
* Anchor text for a {@link PainlessClass}.
*/
private static void emitAnchor(PrintStream stream, PainlessClass struct) {
private static void emitAnchor(PrintStream stream, Class<?> clazz) {
stream.print("painless-api-reference-");
stream.print(struct.name.replace('.', '-'));
stream.print(PainlessLookupUtility.anyTypeToPainlessTypeName(clazz).replace('.', '-'));
}
/**
* Anchor text for a {@link PainlessMethod}.
*/
private static void emitAnchor(PrintStream stream, PainlessMethod method) {
emitAnchor(stream, method.owner);
emitAnchor(stream, method.target);
stream.print('-');
stream.print(methodName(method));
stream.print('-');
@ -226,18 +228,18 @@ public class PainlessDocGenerator {
* Anchor text for a {@link PainlessField}.
*/
private static void emitAnchor(PrintStream stream, PainlessField field) {
emitAnchor(stream, field.owner);
emitAnchor(stream, field.target);
stream.print('-');
stream.print(field.name);
}
private static String methodName(PainlessMethod method) {
return method.name.equals("<init>") ? method.owner.name : method.name;
return method.name.equals("<init>") ? PainlessLookupUtility.anyTypeToPainlessTypeName(method.target) : method.name;
}
/**
* Emit a {@link Class}. If the type is primitive or an array of primitives this just emits the name of the type. Otherwise this emits
an internal link with the text.
an internal link with the text.
*/
private static void emitType(PrintStream stream, Class<?> clazz) {
emitStruct(stream, PAINLESS_LOOKUP.getPainlessStructFromJavaClass(clazz));
@ -253,7 +255,7 @@ public class PainlessDocGenerator {
private static void emitStruct(PrintStream stream, PainlessClass struct) {
if (false == struct.clazz.isPrimitive() && false == struct.name.equals("def")) {
stream.print("<<");
emitAnchor(stream, struct);
emitAnchor(stream, struct.clazz);
stream.print(',');
stream.print(struct.name);
stream.print(">>");
@ -271,14 +273,14 @@ public class PainlessDocGenerator {
stream.print("link:{");
stream.print(root);
stream.print("-javadoc}/");
stream.print(classUrlPath(method.augmentation != null ? method.augmentation : method.owner.clazz));
stream.print(classUrlPath(method.augmentation != null ? method.augmentation : method.target));
stream.print(".html#");
stream.print(methodName(method));
stream.print("%2D");
boolean first = true;
if (method.augmentation != null) {
first = false;
stream.print(method.owner.clazz.getName());
stream.print(method.target.getName());
}
for (Class<?> clazz: method.arguments) {
if (first) {
@ -303,7 +305,7 @@ public class PainlessDocGenerator {
stream.print("link:{");
stream.print(root);
stream.print("-javadoc}/");
stream.print(classUrlPath(field.owner.clazz));
stream.print(classUrlPath(field.target));
stream.print(".html#");
stream.print(field.javaName);
}
@ -315,21 +317,21 @@ public class PainlessDocGenerator {
if (method.augmentation != null) {
return "painless";
}
return javadocRoot(method.owner);
return javadocRoot(method.target);
}
/**
* Pick the javadoc root for a {@link PainlessField}.
*/
private static String javadocRoot(PainlessField field) {
return javadocRoot(field.owner);
return javadocRoot(field.target);
}
/**
* Pick the javadoc root for a {@link PainlessClass}.
* Pick the javadoc root for a {@link Class<?>}.
*/
private static String javadocRoot(PainlessClass struct) {
String classPackage = struct.clazz.getPackage().getName();
private static String javadocRoot(Class<?> clazz) {
String classPackage = clazz.getPackage().getName();
if (classPackage.startsWith("java")) {
return "java8";
}