Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

TestCluster hits AccessControlException in *rules.RamUsageEstimator #41526

Closed
albertzaharovits opened this issue Apr 25, 2019 · 3 comments · Fixed by #49105
Closed

TestCluster hits AccessControlException in *rules.RamUsageEstimator #41526

albertzaharovits opened this issue Apr 25, 2019 · 3 comments · Fixed by #49105
Assignees
@jaymode
Labels
:Core/Infra/Core Core issues without another label >test-failure Triaged test failures from CI

Comments

@albertzaharovits
Copy link
Contributor

Builds
https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+7.x+intake/1185/
and
https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+7.x+multijob-darwin-compatibility/127/

Unreproducible:

/gradlew :modules:reindex:test --tests "org.elasticsearch.index.reindex.ReindexFailureTests.null" \
  -Dtests.seed=BDBD6D0301981618 \
  -Dtests.security.manager=true \
  -Dtests.locale=en-US \
  -Dtests.timezone=UTC \
  -Dcompiler.java=12 \
  -Druntime.java=8

junit.framework.AssertionFailedError: Clean up static fields (in @AfterClass?) and null them, your test still has references to classes of which the sizes cannot be measured due to security restrictions or Java 9 module encapsulation:
  - private static org.elasticsearch.test.TestCluster org.elasticsearch.test.ESIntegTestCase.currentCluster
	at com.carrotsearch.randomizedtesting.rules.StaticFieldsInvariantRule$1.afterAlways(StaticFieldsInvariantRule.java:146)
	at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:43)
	at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
	at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
	at org.apache.lucene.util.TestRuleAssertionsRequired$1.evaluate(TestRuleAssertionsRequired.java:53)
	at org.apache.lucene.util.TestRuleMarkFailure$1.evaluate(TestRuleMarkFailure.java:47)
	at org.apache.lucene.util.TestRuleIgnoreAfterMaxFailures$1.evaluate(TestRuleIgnoreAfterMaxFailures.java:64)
	at org.apache.lucene.util.TestRuleIgnoreTestSuites$1.evaluate(TestRuleIgnoreTestSuites.java:54)
	at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
	at com.carrotsearch.randomizedtesting.ThreadLeakControl$StatementRunner.run(ThreadLeakControl.java:368)
	at java.lang.Thread.run(Thread.java:748)
Caused by: java.security.AccessControlException: access denied ("java.lang.RuntimePermission" "accessClassInPackage.sun.nio.fs")
	at java.security.AccessControlContext.checkPermission(AccessControlContext.java:472)
	at java.security.AccessController.checkPermission(AccessController.java:884)
	at java.lang.SecurityManager.checkPermission(SecurityManager.java:549)
	at java.lang.SecurityManager.checkPackageAccess(SecurityManager.java:1564)
	at java.lang.Class.checkPackageAccess(Class.java:2372)
	at java.lang.Class.checkMemberAccess(Class.java:2351)
	at java.lang.Class.getDeclaredFields(Class.java:1915)
	at com.carrotsearch.randomizedtesting.rules.RamUsageEstimator$2.run(RamUsageEstimator.java:585)
	at com.carrotsearch.randomizedtesting.rules.RamUsageEstimator$2.run(RamUsageEstimator.java:582)
	at java.security.AccessController.doPrivileged(Native Method)
	at com.carrotsearch.randomizedtesting.rules.RamUsageEstimator.createCacheEntry(RamUsageEstimator.java:582)
	at com.carrotsearch.randomizedtesting.rules.RamUsageEstimator.measureSizeOf(RamUsageEstimator.java:545)
	at com.carrotsearch.randomizedtesting.rules.RamUsageEstimator.sizeOfAll(RamUsageEstimator.java:387)
	at com.carrotsearch.randomizedtesting.rules.StaticFieldsInvariantRule$1.afterAlways(StaticFieldsInvariantRule.java:129)
	... 10 more
@albertzaharovits albertzaharovits added :Delivery/Build Build or test infrastructure >test-failure Triaged test failures from CI labels Apr 25, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@albertzaharovits albertzaharovits mentioned this issue Apr 25, 2019
Closed
@jaymode
Copy link
Member

jaymode commented May 2, 2019

We discussed this in the core infra meeting. It appears as something is holding onto to a file/filesystem reference in a static field. We do not want to grant this level of access to the RamUsageEstimator. In order to find out what is holding said reference, we need to look at making an improvement in the RamUsageEstimator and/or the test rule to output the class and even the field name when available.

@alpar-t alpar-t added :Core/Infra/Core Core issues without another label and removed :Delivery/Build Build or test infrastructure labels Oct 23, 2019
@jaymode jaymode self-assigned this Nov 14, 2019
@jaymode
Copy link
Member

jaymode commented Nov 14, 2019

we need to look at making an improvement in the RamUsageEstimator and/or the test rule to output the class and even the field name when available.

This may not be needed as RamUsageEstimator already does output the top level object that triggers the exception, but the information is easily overlooked. In the cases of these failures, the RamUsageEstimator output - private static org.elasticsearch.test.TestCluster org.elasticsearch.test.ESIntegTestCase.currentCluster as the offending static object. This makes sense as the InternalTestCluster class maintains a reference to a set of Path objects that represent data directories and the actual implementation of the Path objects are in the sun.nio.fs package.

In order to reproduce the issue, I was able to create a test that indexed 1000+ documents asynchronously, then performed a full cluster restart using internalCluster().fullRestart(), while indexing was ongoing, and modified the InternalTestCluster to call Node#awaitClose with a value of 0 seconds instead of 10 seconds, which will trigger an exception to be thrown during the execution of ESIntegTestCase#afterClass, which ultimately leads to the static fields not being cleared. JDK8 is necessary for this as LuceneTestCase disables the StaticFieldsInvariantRule if Java 9 or newer is used.

jaymode added a commit to jaymode/elasticsearch that referenced this issue Nov 14, 2019
@jaymode
ESIntegTestCase always cleans up static fields
22fa580 
ESIntegTestCase has logic to clean up static fields in a method
annotated with `@AfterClass` so that these fields do not trigger the
StaticFieldsInvariantRule. However, during the exceptional close of the
test cluster, this cleanup can be missed. The StaticFieldsInvariantRule
always runs and will attempt to inspect the size of the static fields
that were not cleaned up. If the `currentCluster` field of
ESIntegTestCase references an InternalTestCluster, this could hold a
reference to an implementation of a `Path` that comes from the
`sun.nio.fs` package, which the security manager will deny access to.
This casues additional noise to be generated since the
AccessControlException will cause the StaticFieldsInvariantRule to fail
and also be reported along with the actual exception that occurred.

This change clears the static fields of ESIntegTestCase in a finally
block inside the `@AfterClass` method to prevent this unneessary noise.

Closes elastic#41526
@jaymode jaymode mentioned this issue Nov 14, 2019
Merged
jaymode added a commit that referenced this issue Nov 14, 2019
@jaymode
ESIntegTestCase always cleans up static fields (#49105)
ba5b4f1 
ESIntegTestCase has logic to clean up static fields in a method
annotated with `@AfterClass` so that these fields do not trigger the
StaticFieldsInvariantRule. However, during the exceptional close of the
test cluster, this cleanup can be missed. The StaticFieldsInvariantRule
always runs and will attempt to inspect the size of the static fields
that were not cleaned up. If the `currentCluster` field of
ESIntegTestCase references an InternalTestCluster, this could hold a
reference to an implementation of a `Path` that comes from the
`sun.nio.fs` package, which the security manager will deny access to.
This casues additional noise to be generated since the
AccessControlException will cause the StaticFieldsInvariantRule to fail
and also be reported along with the actual exception that occurred.

This change clears the static fields of ESIntegTestCase in a finally
block inside the `@AfterClass` method to prevent this unnecessary noise.

Closes #41526
@jaymode jaymode mentioned this issue Nov 14, 2019
Merged
jaymode added a commit that referenced this issue Nov 15, 2019
@jaymode
ESIntegTestCase always cleans up static fields (#49105) (#49108)
b6ec066 
ESIntegTestCase has logic to clean up static fields in a method
annotated with `@AfterClass` so that these fields do not trigger the
StaticFieldsInvariantRule. However, during the exceptional close of the
test cluster, this cleanup can be missed. The StaticFieldsInvariantRule
always runs and will attempt to inspect the size of the static fields
that were not cleaned up. If the `currentCluster` field of
ESIntegTestCase references an InternalTestCluster, this could hold a
reference to an implementation of a `Path` that comes from the
`sun.nio.fs` package, which the security manager will deny access to.
This casues additional noise to be generated since the
AccessControlException will cause the StaticFieldsInvariantRule to fail
and also be reported along with the actual exception that occurred.

This change clears the static fields of ESIntegTestCase in a finally
block inside the `@AfterClass` method to prevent this unnecessary noise.

Closes #41526
@jaymode jaymode mentioned this issue Nov 15, 2019
Merged
jaymode added a commit to jaymode/elasticsearch that referenced this issue Nov 15, 2019
@jaymode
Clean up static web server in sql-client tests
e9e8a48 
The JdbcHttpClientRequestTests and HttpClientRequestTests classes both
hold a static reference to a mock web server that internally uses the
JDKs built-in HttpServer, which resides in a sun package that the
RamUsageEstimator does not have access to. This causes builds that use
a runtime of Java 8 to fail since the StaticFieldsInvariantRule is run
when Java 8 is used.

Relates elastic#41526
Relates elastic#49105
jaymode added a commit that referenced this issue Nov 15, 2019
@jaymode
Clean up static web server in sql-client tests (#49187)
213d2da 
The JdbcHttpClientRequestTests and HttpClientRequestTests classes both
hold a static reference to a mock web server that internally uses the
JDKs built-in HttpServer, which resides in a sun package that the
RamUsageEstimator does not have access to. This causes builds that use
a runtime of Java 8 to fail since the StaticFieldsInvariantRule is run
when Java 8 is used.

Relates #41526
Relates #49105
@jaymode jaymode mentioned this issue Nov 15, 2019
Merged
jaymode added a commit that referenced this issue Nov 15, 2019
@jaymode
Clean up static web server in sql-client tests (#49187) (#49197)
57f5722 
The JdbcHttpClientRequestTests and HttpClientRequestTests classes both
hold a static reference to a mock web server that internally uses the
JDKs built-in HttpServer, which resides in a sun package that the
RamUsageEstimator does not have access to. This causes builds that use
a runtime of Java 8 to fail since the StaticFieldsInvariantRule is run
when Java 8 is used.

Relates #41526
Relates #49105
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jaymode jaymode

Labels
:Core/Infra/Core Core issues without another label >test-failure Triaged test failures from CI
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

ESIntegTestCase always cleans up static fields jaymode/elasticsearch
4 participants
@alpar-t @jaymode @albertzaharovits @elasticmachine