Skip to content

Commit 8f2aebf

Browse files
anubhavi25anubhavi25
committed
Fix the metaspace leak that occurs every time a policy is evaluated or a policy set is created
1 parent 95ecbba commit 8f2aebf

File tree

16 files changed

+215
-214
lines changed

16 files changed

+215
-214
lines changed

‎acs-integration-tests/pom.xml

+2-1
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@
7474
<plugin>
7575
<groupId>org.codehaus.cargo</groupId>
7676
<artifactId>cargo-maven2-plugin</artifactId>
77-
<version>1.4.19</version>
77+
<version>1.7.7</version>
7878
<configuration>
7979
<skip>${maven.test.skip}</skip>
8080
<configuration>
@@ -84,6 +84,7 @@
8484
</properties>
8585
</configuration>
8686
<container>
87+
<timeout>600000</timeout>
8788
<containerId>tomcat8x</containerId>
8889
<systemProperties>
8990
<CLOUD_FOUNDRY_CONFIG_PATH>${project.basedir}/uaa/config</CLOUD_FOUNDRY_CONFIG_PATH>

‎commons/pom.xml

+2-1
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@
6161
</dependency>
6262
<dependency>
6363
<groupId>org.codehaus.groovy</groupId>
64-
<artifactId>groovy-all</artifactId>
64+
<artifactId>groovy</artifactId>
65+
<classifier>indy</classifier>
6566
</dependency>
6667
<dependency>
6768
<groupId>org.springframework</groupId>

‎commons/src/main/java/org/eclipse/keti/acs/commons/policy/condition/ConditionShell.java

-53
This file was deleted.

‎commons/src/main/java/org/eclipse/keti/acs/commons/policy/condition/groovy/GroovyConditionScript.java

-1
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ public class GroovyConditionScript implements ConditionScript {
4444
* the script object.
4545
*/
4646
public GroovyConditionScript(final Script script) {
47-
super();
4847
this.script = script;
4948
}
5049

‎commons/src/main/java/org/eclipse/keti/acs/commons/policy/condition/groovy/GroovyConditionShell.java

+60-53
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@
2323

2424
import java.util.Arrays;
2525
import java.util.Collection;
26+
import java.util.Collections;
2627
import java.util.Map;
27-
import java.util.Map.Entry;
2828
import java.util.Set;
2929

3030
import org.apache.commons.lang.StringUtils;
@@ -37,14 +37,9 @@
3737
import org.eclipse.keti.acs.commons.policy.condition.ConditionAssertionFailedException;
3838
import org.eclipse.keti.acs.commons.policy.condition.ConditionParsingException;
3939
import org.eclipse.keti.acs.commons.policy.condition.ConditionScript;
40-
import org.eclipse.keti.acs.commons.policy.condition.ConditionShell;
4140
import org.eclipse.keti.acs.commons.policy.condition.ResourceHandler;
4241
import org.eclipse.keti.acs.commons.policy.condition.SubjectHandler;
43-
import org.slf4j.Logger;
44-
import org.slf4j.LoggerFactory;
45-
import org.springframework.stereotype.Component;
4642

47-
import groovy.lang.Binding;
4843
import groovy.lang.GroovyShell;
4944
import groovy.lang.GroovySystem;
5045
import groovy.lang.Script;
@@ -54,13 +49,13 @@
5449
* @author [email protected]
5550
*/
5651
@SuppressWarnings("nls")
57-
@Component
58-
public class GroovyConditionShell implements ConditionShell {
59-
private static final Logger LOGGER = LoggerFactory.getLogger(GroovyConditionShell.class);
52+
public class GroovyConditionShell {
6053

54+
private final GroovyConditionCache conditionCache;
6155
private final GroovyShell shell;
6256

63-
public GroovyConditionShell() {
57+
public GroovyConditionShell(final GroovyConditionCache conditionCache) {
58+
this.conditionCache = conditionCache;
6459

6560
SecureASTCustomizer secureASTCustomizer = createSecureASTCustomizer();
6661
ImportCustomizer importCustomizer = createImportCustomizer();
@@ -70,66 +65,78 @@ public GroovyConditionShell() {
7065
compilerConfiguration.addCompilationCustomizers(secureASTCustomizer);
7166
compilerConfiguration.addCompilationCustomizers(importCustomizer);
7267
compilerConfiguration.addCompilationCustomizers(astTransformationCustomizer);
68+
compilerConfiguration.getOptimizationOptions().put(CompilerConfiguration.INVOKEDYNAMIC, true);
7369

74-
this.shell = new GroovyShell(GroovyConditionShell.class.getClassLoader(), compilerConfiguration);
70+
this.shell = new GroovyShell(this.getClass().getClassLoader(), compilerConfiguration);
7571
}
7672

77-
/*
78-
* (non-Javadoc)
73+
/**
74+
* Validates the script & generates condition script object.
7975
*
80-
* @see org.eclipse.keti.acs.commons.conditions.ConditionShell#parse(java.lang. String )
76+
* @param script
77+
* the policy condition string
78+
* @return a Script object instance capable of executing the policy condition.
79+
* @throws ConditionParsingException
80+
* on validation error
8181
*/
82-
@Override
8382
public ConditionScript parse(final String script) throws ConditionParsingException {
83+
return parse(script, true);
84+
}
85+
86+
private ConditionScript parse(
87+
final String script,
88+
final boolean removeLoadedClasses
89+
) throws ConditionParsingException {
8490
if (StringUtils.isEmpty(script)) {
8591
throw new IllegalArgumentException("Script is null or empty.");
8692
}
8793

8894
try {
89-
Script groovyScript = this.shell.parse(script);
90-
return new GroovyConditionScript(groovyScript);
95+
ConditionScript compiledScript = conditionCache.get(script);
96+
if (compiledScript == null) {
97+
Script groovyScript = this.shell.parse(script);
98+
compiledScript = new GroovyConditionScript(groovyScript);
99+
conditionCache.put(script, compiledScript);
100+
}
101+
return compiledScript;
91102
} catch (CompilationFailedException e) {
92-
throw new ConditionParsingException("Failed to validate the condition script.", script, e);
103+
throw new ConditionParsingException("Failed to parse the condition script.", script, e);
104+
} finally {
105+
if (removeLoadedClasses) {
106+
removeLoadedClasses();
107+
}
93108
}
94109
}
95110

96-
/*
97-
* (non-Javadoc)
111+
/**
112+
* Validates & executes the policy condition script.
98113
*
99-
* @see org.eclipse.keti.acs.commons.conditions.ConditionShell#execute(java.lang. String, java.util.Map )
114+
* @param script
115+
* the policy condition string
116+
* @param boundVariables
117+
* variable bindings of the script
118+
* @return result of executing the policy condition script.
119+
* @throws ConditionParsingException
120+
* on script validation error
100121
*/
101-
@Override
102122
public boolean execute(final String script, final Map<String, Object> boundVariables)
103123
throws ConditionParsingException {
104-
if (StringUtils.isEmpty(script)) {
105-
throw new IllegalArgumentException("Script is null or empty.");
106-
}
124+
ConditionScript conditionScript = parse(script, false);
107125

108126
try {
109-
Script groovyScript = this.shell.parse(script);
110-
111-
if (LOGGER.isDebugEnabled()) {
112-
StringBuilder msgBuilder = new StringBuilder();
113-
msgBuilder.append("The script is bound to the following variables:\n");
114-
for (Entry<String, Object> entry : boundVariables.entrySet()) {
115-
msgBuilder.append("* ").append(entry.getKey()).append(":").append(entry.getValue()).append("\n");
116-
}
117-
LOGGER.debug(msgBuilder.toString());
118-
}
119-
120-
Binding binding = new Binding(boundVariables);
121-
groovyScript.setBinding(binding);
122-
boolean result = (boolean) groovyScript.run();
123-
124-
this.shell.getClassLoader().clearCache();
125-
GroovySystem.getMetaClassRegistry().removeMetaClass(groovyScript.getClass());
126-
127-
return result;
128-
} catch (CompilationFailedException e) {
129-
throw new ConditionParsingException("Failed to parse the condition script.", script, e);
127+
return conditionScript.execute(boundVariables);
130128
} catch (ConditionAssertionFailedException e) {
131129
return false;
130+
} finally {
131+
removeLoadedClasses();
132+
}
133+
}
134+
135+
private void removeLoadedClasses() {
136+
for (Class<?> groovyClass : shell.getClassLoader().getLoadedClasses()) {
137+
GroovySystem.getMetaClassRegistry().removeMetaClass(groovyClass);
132138
}
139+
shell.resetLoadedClasses();
133140
}
134141

135142
private static ImportCustomizer createImportCustomizer() {
@@ -147,18 +154,18 @@ private static SecureASTCustomizer createSecureASTCustomizer() {
147154
// Disallow method definition.
148155
secureASTCustomizer.setMethodDefinitionAllowed(false);
149156
// Disallow all imports by setting a blank whitelist.
150-
secureASTCustomizer.setImportsWhitelist(Arrays.asList(new String[] {}));
157+
secureASTCustomizer.setImportsWhitelist(Collections.emptyList());
151158
// Disallow star imports by setting a blank whitelist.
152159
secureASTCustomizer.setStarImportsWhitelist(Arrays.asList(
153-
new String[] { "org.crsh.command.*", "org.crsh.cli.*", "org.crsh.groovy.*",
154-
"org.eclipse.keti.acs.commons.policy.condition.*" }));
160+
"org.crsh.command.*", "org.crsh.cli.*", "org.crsh.groovy.*",
161+
"org.eclipse.keti.acs.commons.policy.condition.*"));
155162
// Set white list for constant type classes.
156163
secureASTCustomizer.setConstantTypesClassesWhiteList(Arrays.asList(
157-
new Class[] { Boolean.class, boolean.class, Collection.class, Double.class, double.class, Float.class,
158-
float.class, Integer.class, int.class, Long.class, long.class, Object.class, String.class }));
164+
Boolean.class, boolean.class, Collection.class, Double.class, double.class, Float.class,
165+
float.class, Integer.class, int.class, Long.class, long.class, Object.class, String.class));
159166
secureASTCustomizer.setReceiversClassesWhiteList(Arrays.asList(
160-
new Class[] { Boolean.class, Collection.class, Integer.class, Iterable.class, Object.class, Set.class,
161-
String.class }));
167+
Boolean.class, Collection.class, Integer.class, Iterable.class, Object.class, Set.class,
168+
String.class));
162169
return secureASTCustomizer;
163170
}
164171

‎commons/src/test/java/org/eclipse/keti/acs/commons/policy/condition/groovy/GroovyConditionScriptTest.java

+1-8
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,10 @@
2323
import java.util.Map;
2424

2525
import org.testng.Assert;
26-
import org.testng.annotations.BeforeClass;
2726
import org.testng.annotations.Test;
2827

2928
import org.eclipse.keti.acs.commons.policy.condition.ConditionParsingException;
3029
import org.eclipse.keti.acs.commons.policy.condition.ConditionScript;
31-
import org.eclipse.keti.acs.commons.policy.condition.ConditionShell;
3230
import org.eclipse.keti.acs.commons.policy.condition.ResourceHandler;
3331
import org.eclipse.keti.acs.commons.policy.condition.SubjectHandler;
3432
import org.eclipse.keti.acs.model.Attribute;
@@ -39,12 +37,7 @@
3937
* @author [email protected]
4038
*/
4139
public class GroovyConditionScriptTest {
42-
private ConditionShell shell;
43-
44-
@BeforeClass
45-
public void setup() {
46-
this.shell = new GroovyConditionShell();
47-
}
40+
private final GroovyConditionShell shell = new GroovyConditionShell(new NonCachingGroovyConditionCache());
4841

4942
/**
5043
* Test the execution of a policy condition, which should evaluate to true, using strings constants.

‎commons/src/test/java/org/eclipse/keti/acs/commons/policy/condition/groovy/GroovyConditionShellTest.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424

2525
import org.eclipse.keti.acs.commons.policy.condition.ConditionParsingException;
2626
import org.eclipse.keti.acs.commons.policy.condition.ConditionScript;
27-
import org.eclipse.keti.acs.commons.policy.condition.ConditionShell;
2827
import org.eclipse.keti.acs.commons.policy.condition.ResourceHandler;
2928
import org.eclipse.keti.acs.commons.policy.condition.SubjectHandler;
3029
import org.eclipse.keti.acs.model.Attribute;
@@ -44,7 +43,7 @@ public class GroovyConditionShellTest {
4443

4544
private static Map<String, Object> emptyBindingMap = new HashMap<>();
4645

47-
private ConditionShell shell = new GroovyConditionShell();
46+
private final GroovyConditionShell shell = new GroovyConditionShell(new NonCachingGroovyConditionCache());
4847

4948
/**
5049
* Test a policy condition parsing and compilation using an allowed policy operations.

0 commit comments

Comments
 (0)