Skip to content

Commit

Permalink
Script: track this pointer capture from blocks within lambdas (elasti…
Browse files Browse the repository at this point in the history
…c#82228)

* Script: track this pointer capture from blocks within lambdas

If a lambda contains a block that calls into a user function,
the painless compiler was not tracking that the lambda needs
to capture the `this` pointer.

Scripts that attempted to use a lambda that calls a user
function from inside a block would trigger an
`illegal_state_exception`:

> no 'this' pointer within static method

`BlockScope` now forwards `setUsesInstanceMethod` calls to
it's parent, which may be a `LambdaScope`.

`LambdaScope` will also forward `setUsesInstanceMethod` to
it's parent after setting it's own `usesInstanceMethod` flag,
propagating `this` pointer capture in the case of nested lambdas.

Fixes: elastic#82224
  • Loading branch information
stu-elastic committed Jan 4, 2022
1 parent 0854d44 commit 896e968
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 1 deletion.
5 changes: 5 additions & 0 deletions docs/changelog/82228.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 82228
summary: "Script: track this pointer capture from blocks within lambdas"
area: Infra/Scripting
type: bug
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,9 @@ public void setUsesInstanceMethod() {
return;
}
usesInstanceMethod = true;
if (parent != null) {
parent.setUsesInstanceMethod();
}
}

@Override
Expand Down Expand Up @@ -261,6 +264,12 @@ public Class<?> getReturnType() {
public String getReturnCanonicalTypeName() {
return parent.getReturnCanonicalTypeName();
}

@Override
// If the parent scope is a lambda, we want to track this usage, so forward call to parent.
public void setUsesInstanceMethod() {
parent.setUsesInstanceMethod();
}
}

/**
Expand Down Expand Up @@ -356,7 +365,8 @@ public Variable defineVariable(Location location, Class<?> type, String name, bo

public abstract Variable getVariable(Location location, String name);

// We only want to track instance method use inside of lambdas for "this" injection. It's a noop for other scopes.
// We only want to track instance method use inside of lambdas (and blocks inside lambdas) for "this" injection.
// It's a noop for other scopes.
public void setUsesInstanceMethod() {}

public boolean usesInstanceMethod() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,4 +140,40 @@ public void testLambdaCapture() {
assertEquals(org.elasticsearch.core.List.of(100, 1, -100), exec(source, org.elasticsearch.core.Map.of("a", 1), false));
assertBytecodeExists(source, "public static synthetic lambda$synthetic$0(ILjava/lang/Object;Ljava/lang/Object;)I");
}

public void testCallUserMethodFromStatementWithinLambda() {
String source = ""
+ "int test1() { return 1; }"
+ "void test(Map params) { "
+ " int i = 0;"
+ " params.forEach("
+ " (k, v) -> { if (i == 0) { test1() } else { 20 } }"
+ " );"
+ "}"
+ "test(params)";
assertNull(exec(source, Map.of("a", 5), false));
assertBytecodeExists(source, "public synthetic lambda$synthetic$0(ILjava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;");
}

public void testCallUserMethodFromStatementWithinNestedLambda() {
String source = ""
+ "int test1() { return 1; }"
+ "void test(Map params) { "
+ " int i = 0;"
+ " int j = 5;"
+ " params.replaceAll( "
+ " (n, m) -> {"
+ " m.forEach("
+ " (k, v) -> { if (i == 0) { test1() } else { 20 } }"
+ " );"
+ " return ['aaa': j];"
+ " }"
+ " );"
+ "}"
+ "Map myParams = new HashMap(params);"
+ "test(myParams);"
+ "myParams['a']['aaa']";
assertEquals(5, exec(source, Map.of("a", Map.of("b", 1)), false));
assertBytecodeExists(source, "public synthetic lambda$synthetic$1(IILjava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;");
}
}

0 comments on commit 896e968

Please sign in to comment.