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.

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

ESIntegTestCase always cleans up static fields #49105

Merged

Conversation

jaymode
Copy link
Member

@jaymode jaymode commented Nov 14, 2019

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

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 added >test Issues or PRs that are addressing/adding tests :Core/Infra/Core Core issues without another label v8.0.0 v7.6.0 labels Nov 14, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (:Core/Infra/Core)

Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@jaymode jaymode merged commit ba5b4f1 into elastic:master Nov 14, 2019
@jaymode jaymode deleted the ram_usage_estim_access_denied_sun_nio_fs branch November 14, 2019 20:08
jaymode added a commit that referenced this pull request Nov 15, 2019
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 added a commit to jaymode/elasticsearch that referenced this pull request Nov 15, 2019
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 pull request Nov 15, 2019
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 added a commit that referenced this pull request Nov 15, 2019
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
Labels
:Core/Infra/Core Core issues without another label >test Issues or PRs that are addressing/adding tests v7.6.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TestCluster hits AccessControlException in *rules.RamUsageEstimator
5 participants