Skip to content

Commit

Permalink
Merge pull request #41828 from MaryamZi/fix-isolated-analysis
Browse files Browse the repository at this point in the history 
Fix isolated analysis when accessing `self` of an isolated object within an anonymous function
  • Loading branch information
@MaryamZi
MaryamZi authored Dec 6, 2023
2 parents 5ae9d10 + f798dd4 commit bcb4a27
Show file tree
Hide file tree
Showing 7 changed files with 266 additions and 9 deletions.
45 changes: 38 additions & 7 deletions ...g/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/IsolationAnalyzer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2150,8 +2150,7 @@ private BLangInvokableNode getWorkerEnclosedFunctionInvokable() {
SymbolEnv env = this.env;
while (env != null) {
BLangNode node = env.node;
if (node != null && (node.getKind() == NodeKind.FUNCTION || node.getKind() == NodeKind.RESOURCE_FUNC) &&
!isWorkerLambda((BLangFunction) node)) {
if (node != null && isFunction(node) && !isWorkerLambda((BLangFunction) node)) {
return env.enclInvokable;
}
env = env.enclEnv;
Expand Down Expand Up @@ -2605,7 +2604,8 @@ private boolean isInvalidIsolatedObjectFieldOrMethodAccessViaSelfIfOutsideLock(
return false;
}

BField field = ((BObjectType) env.enclInvokable.symbol.owner.type).fields.get(fieldAccessExpr.field.value);
BLangInvokableNode enclInvokable = getEnclNonAnonymousFunction((BLangFunction) env.enclInvokable);
BField field = ((BObjectType) enclInvokable.symbol.owner.type).fields.get(fieldAccessExpr.field.value);

if (field == null) {
// Bound method access.
Expand Down Expand Up @@ -3287,13 +3287,21 @@ private boolean isDefinedOutsideLock(Name name, long symTag, SymbolEnv currentEn
private boolean isInIsolatedObjectMethod(SymbolEnv env, boolean ignoreInit) {
BLangInvokableNode enclInvokable = env.enclInvokable;

if (enclInvokable == null ||
(enclInvokable.getKind() != NodeKind.FUNCTION && enclInvokable.getKind() != NodeKind.RESOURCE_FUNC)) {
if (enclInvokable == null || !isFunction(enclInvokable)) {
return false;
}

BLangFunction enclFunction = (BLangFunction) enclInvokable;

if (Symbols.isFlagOn(enclFunction.symbol.flags, Flags.ANONYMOUS)) {
BLangFunction enclNonAnonymousFunction = getEnclNonAnonymousFunction(enclFunction);

if (enclNonAnonymousFunction == null) {
return false;
}
enclFunction = enclNonAnonymousFunction;
}

if (!enclFunction.attachedFunction) {
return false;
}
Expand All @@ -3302,11 +3310,34 @@ private boolean isInIsolatedObjectMethod(SymbolEnv env, boolean ignoreInit) {
return false;
}

BType ownerType = Types.getImpliedType(enclInvokable.symbol.owner.type);
BType ownerType = Types.getImpliedType(enclFunction.symbol.owner.type);

return ownerType.tag == TypeTags.OBJECT && isIsolated(ownerType.flags);
}

private BLangFunction getEnclNonAnonymousFunction(BLangFunction enclFunction) {
if (enclFunction == null || enclFunction.symbol.owner.kind == SymbolKind.PACKAGE) {
return null;
}

if (!Symbols.isFlagOn(enclFunction.symbol.flags, Flags.ANONYMOUS)) {
return enclFunction;
}

BLangNode enclNode = enclFunction.parent;

while (enclNode != null && !isFunction(enclNode)) {
enclNode = enclNode.parent;
}

return getEnclNonAnonymousFunction((BLangFunction) enclNode);
}

private boolean isFunction(BLangNode enclNode) {
NodeKind kind = enclNode.getKind();
return kind == NodeKind.FUNCTION || kind == NodeKind.RESOURCE_FUNC;
}

private boolean isInvalidCopyIn(BLangSimpleVarRef varRefExpr, SymbolEnv currentEnv) {
return isInvalidCopyIn(varRefExpr, names.fromIdNode(varRefExpr.variableName), varRefExpr.symbol.tag,
currentEnv);
Expand Down Expand Up @@ -3375,7 +3406,7 @@ private boolean isEnclosedLockWithinSameFunction(BLangNode parent, BLangLock pot
return true;
}

if (parent == null || parent.getKind() == NodeKind.FUNCTION || parent.getKind() == NodeKind.RESOURCE_FUNC) {
if (parent == null || isFunction(parent)) {
return false;
}

Expand Down
8 changes: 8 additions & 0 deletions ...allerina-unit-test/src/test/java/org/ballerinalang/test/isolation/IsolatedObjectTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,14 @@ public void testIsolatedObjectIsolationNegative() {
validateError(result, i++, ERROR_INVALID_NON_ISOLATED_INVOCATION_IN_LOCK_WITH_RESTRICTED_VAR_USAGE, 865, 27);
validateError(result, i++, ERROR_INVALID_TRANSFER_IN_IN_LOCK_WITH_RESTRICTED_VAR_USAGE, 880, 31);
validateError(result, i++, ERROR_INVALID_NON_ISOLATED_INVOCATION_IN_LOCK_WITH_RESTRICTED_VAR_USAGE, 881, 32);
validateError(result, i++, INVALID_ACCESS_OF_ISOLATED_OBJECT_MUTABLE_FIELD_OUTSIDE_A_LOCK_STATEMENT, 893, 13);
validateError(result, i++, INVALID_ACCESS_OF_ISOLATED_OBJECT_MUTABLE_FIELD_OUTSIDE_A_LOCK_STATEMENT, 899, 13);
validateError(result, i++, INVALID_ACCESS_OF_ISOLATED_OBJECT_MUTABLE_FIELD_OUTSIDE_A_LOCK_STATEMENT, 911, 29);
validateError(result, i++, ERROR_INVALID_TRANSFER_IN_IN_LOCK_WITH_RESTRICTED_VAR_USAGE, 926, 31);
validateError(result, i++, ERROR_INVALID_TRANSFER_OUT_IN_LOCK_WITH_RESTRICTED_VAR_USAGE, 934, 24);
validateError(result, i++, ERROR_INVALID_TRANSFER_IN_IN_LOCK_WITH_RESTRICTED_VAR_USAGE, 948, 52);
validateError(result, i++, INVALID_ACCESS_OF_ISOLATED_OBJECT_MUTABLE_FIELD_OUTSIDE_A_LOCK_STATEMENT, 963, 13);
validateError(result, i++, ERROR_INVALID_TRANSFER_IN_IN_LOCK_WITH_RESTRICTED_VAR_USAGE, 970, 31);
Assert.assertEquals(result.getErrorCount(), i - 19);
Assert.assertEquals(result.getWarnCount(), 19);
}
Expand Down
3 changes: 2 additions & 1 deletion ...rina-unit-test/src/test/java/org/ballerinalang/test/isolation/IsolationInferenceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ public Object[] isolationInferenceTests() {
"testFunctionsAccessingModuleLevelVarsIsolatedInference",
"testFunctionCallingFunctionWithIsolatedParamAnnotatedParam",
"testInferringIsolatedForAnonFuncArgForIsolatedParamAnnotatedParam",
"testIsolatedObjectsWithNonInitializationSelfAccessInInitMethod"
"testIsolatedObjectsWithNonInitializationSelfAccessInInitMethod",
"testIsolatedInferenceWithAnonFunctions"
};
}

Expand Down
50 changes: 50 additions & 0 deletions tests/jballerina-unit-test/src/test/resources/test-src/isolated-objects/isolated_objects.bal
View file
Original file line number Diff line number Diff line change
Expand Up @@ -891,6 +891,56 @@ function testLocalObjectConstructorWithValidAccessInInitMethodAfterInitializatio
};
}

isolated class TestIsolatedObjectWithSelfAccessInAnonFunctions {
private int[][] arr = [];

function init(int[] node) {
function _ = function () {
lock {
self.arr.push(node.clone());
}
};
}

function f1(int[] node) {
function _ = function () returns int[] {
lock {
return self.arr[0].cloneReadOnly();
}
};
}

function f2(int[] node) {
lock {
function _ = function () {
object {} _ = isolated object {
private int[][] innerArr = [];

function innerF(int[] innerNode) {
function _ = function () {
lock {
self.innerArr.push(innerNode.clone());
}
};
}
};
};
}
}

function f3() {
object{} _ = object {
private int[][] arr2 = [];

function fn() {
function _ = function (int[] node) {
self.arr2.push(node); // OK because `self` is of the non-isolated object
};
}
};
}
}

function assertTrue(any|error actual) {
assertEquality(true, actual);
}
Expand Down
88 changes: 88 additions & 0 deletions ...test/src/test/resources/test-src/isolated-objects/isolated_objects_isolation_negative.bal
View file
Original file line number Diff line number Diff line change
Expand Up @@ -884,3 +884,91 @@ function testIsolatedObjectConstructorWithInvalidInitMethod() {
}
};
}

isolated class TestInvalidAccessOfIsolatedObjectSelfInAnonFunctions {
private int[][] arr = [];

function init(int[] node) {
function _ = function () {
self.arr.push(node);
};
}

function f1(int[] node) {
function _ = function () {
self.arr.push(node);
};
}

function f2(int[] node) {
lock {
function _ = function () {
object {} _ = isolated object {
private int[][] innerArr = [];

function innerF(int[] innerNode) {
function _ = function () {
self.innerArr.push(innerNode);
};
}
};
};
}
}
}

isolated class TestInvalidTransferOfValuesInAnonFunctionsInIsolatedObject {
private int[][] arr = [];

function init(int[] node) {
function _ = function () {
lock {
self.arr.push(node);
}
};
}

function f1(int[] node) {
function _ = function () returns int[] {
lock {
return self.arr[0];
}
};
}

function f2(int[] node) {
lock {
function _ = function () {
object {} _ = isolated object {
private int[][] innerArr = [];

function innerF(int[] innerNode) {
function _ = function () {
lock {
self.innerArr.push(innerNode);
}
};
}
};
};
}
}
}

client isolated class TestInvalidAccessOfSelfWithinAnonFunctionInRemoteAndResourceMethods {
private int[][] arr = [];

resource function get test() {
function _ = function (int[] node) {
self.arr.push(node);
};
}

remote function testFn() {
function _ = function (int[] node) {
lock {
self.arr.push(node);
}
};
}
}
6 changes: 5 additions & 1 deletion ...ballerina-unit-test/src/test/resources/test-src/isolation-analysis/isolation_analysis.bal
View file
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,11 @@ isolated class IsolatedFunctionWithIsolatedSelfAsCapturedVariable {

isolated function compare() returns boolean {
lock {
return self.words.some(isolated function (string s) returns boolean => s.length() > self.min);
return self.words.some(isolated function (string s) returns boolean {
lock {
return s.length() > self.min;
}
});
}
}
}
Expand Down
75 changes: 75 additions & 0 deletions ...allerina-unit-test/src/test/resources/test-src/isolation-analysis/isolation_inference.bal
View file
Original file line number Diff line number Diff line change
Expand Up @@ -773,6 +773,81 @@ function testIsolatedObjectsWithNonInitializationSelfAccessInInitMethod() {
assertTrue(<any> z is isolated object {});
}

class ObjectWithSelfAccessInLocksInAnonFunctions {
private int[][] arr = [];

function init(int[] node) {
function _ = function () {
lock {
self.arr.push(node.clone());
}
};
}

function f1(int[] node) {
function _ = function () returns int[] {
lock {
return self.arr[0].cloneReadOnly();
}
};
}

function f2(int[] node) returns object {} {
lock {
var fn = function () returns object {} {
return isolated object {
private int[][] innerArr = [];

function innerF(int[] innerNode) {
function _ = function () {
lock {
self.innerArr.push(innerNode.clone());
}
};
}
};
};
return fn();
}
}
}

class ObjectWithSelfAccessOutsideLocksInAnonFunctions {
private int[][] arr = [];

function f1(int[] node) {
function _ = function () returns int[] {
return self.arr[0].cloneReadOnly();
};
}
}

class ObjectWithInvalidTransferInAnonFunctions {
private int[][] arr = [];

function f1(int[] node) {
function _ = function () returns int[] {
lock {
return self.arr[0];
}
};
}
}

function testIsolatedInferenceWithAnonFunctions() {
ObjectWithSelfAccessInLocksInAnonFunctions a = new ([]);
assertTrue(<any> a is isolated object {});

object {} b = a.f2([]);
assertTrue(<any> b is isolated object {});

any c = new ObjectWithSelfAccessOutsideLocksInAnonFunctions();
assertFalse(c is isolated object {});

any d = new ObjectWithInvalidTransferInAnonFunctions();
assertFalse(d is isolated object {});
}

isolated function assertTrue(anydata actual) => assertEquality(true, actual);

isolated function assertFalse(anydata actual) => assertEquality(false, actual);
Expand Down

0 comments on commit bcb4a27

Please sign in to comment.