Skip to content

Remove PrivilegedOperations #127726

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

Merged
merged 4 commits into from
May 6, 2025
Merged

Remove PrivilegedOperations #127726

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0332594
Remove PrivilegedOperations
rjernst May 5, 2025
2fb8baf
[CI] Auto commit changes from spotless
May 5, 2025
4faac1c
fix compile
rjernst May 6, 2025
181c932
Merge branch 'main' into sm/remove_privileged_operations
rjernst May 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 13 additions & 27 deletions .../src/test/java/org/elasticsearch/core/internal/provider/EmbeddedImplClassLoaderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@

package org.elasticsearch.core.internal.provider;

import org.elasticsearch.core.IOUtils;
import org.elasticsearch.core.Strings;
import org.elasticsearch.core.SuppressForbidden;
import org.elasticsearch.core.internal.provider.EmbeddedImplClassLoader.CompoundEnumeration;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.PrivilegedOperations;
import org.elasticsearch.test.compiler.InMemoryJavaCompiler;
import org.elasticsearch.test.jar.JarUtils;

Expand Down Expand Up @@ -195,13 +195,10 @@ private Object newFooBar(boolean enableMulti, int... versions) throws Exception
Path outerJar = topLevelDir.resolve("impl.jar");
JarUtils.createJarWithEntries(outerJar, jarEntries);
URL[] urls = new URL[] { outerJar.toUri().toURL() };
URLClassLoader parent = URLClassLoader.newInstance(urls, EmbeddedImplClassLoaderTests.class.getClassLoader());
try {
try (URLClassLoader parent = loader(urls)) {
EmbeddedImplClassLoader loader = EmbeddedImplClassLoader.getInstance(parent, "x-foo");
Class<?> c = loader.loadClass("p.FooBar");
return c.getConstructor().newInstance();
} finally {
PrivilegedOperations.closeURLClassLoader(parent);
}
}

Expand Down Expand Up @@ -245,8 +242,7 @@ public void testResourcesBasic() throws Exception {
Path outerJar = topLevelDir.resolve("impl.jar");
JarUtils.createJarWithEntriesUTF(outerJar, jarEntries);
URL[] urls = new URL[] { outerJar.toUri().toURL() };
URLClassLoader parent = URLClassLoader.newInstance(urls, EmbeddedImplClassLoaderTests.class.getClassLoader());
try {
try (URLClassLoader parent = loader(urls)) {
EmbeddedImplClassLoader loader = EmbeddedImplClassLoader.getInstance(parent, "res");
// resource in a valid java package dir
URL url = loader.findResource("p/res.txt");
Expand Down Expand Up @@ -274,8 +270,6 @@ public void testResourcesBasic() throws Exception {
hasToString(endsWith("impl.jar!/IMPL-JARS/res/zoo-impl.jar/A-C/res.txt"))
)
);
} finally {
PrivilegedOperations.closeURLClassLoader(parent);
}
}

Expand Down Expand Up @@ -326,9 +320,7 @@ private void testResourcesParent(String resourcePath) throws Exception {
containsInAnyOrder("Parent Resource", "Embedded Resource")
);
} finally {
for (URLClassLoader closeable : closeables) {
PrivilegedOperations.closeURLClassLoader(closeable);
}
IOUtils.close(closeables);
}
}

Expand Down Expand Up @@ -463,9 +455,7 @@ private void testResourcesVersioned(String resourcePath, boolean enableMulti, in
assertThat(new String(is.readAllBytes(), UTF_8), is("Hello World" + expectedVersion));
}
} finally {
for (URLClassLoader closeable : closeables) {
PrivilegedOperations.closeURLClassLoader(closeable);
}
IOUtils.close(closeables);
}
}

Expand Down Expand Up @@ -493,8 +483,7 @@ public void testIDontHaveIt() throws Exception {
Path outerJar = topLevelDir.resolve("impl.jar");
JarUtils.createJarWithEntriesUTF(outerJar, jarEntries);
URL[] urls = new URL[] { outerJar.toUri().toURL() };
URLClassLoader parent = URLClassLoader.newInstance(urls, EmbeddedImplClassLoaderTests.class.getClassLoader());
try {
try (URLClassLoader parent = loader(urls)) {
embedLoader = EmbeddedImplClassLoader.getInstance(parent, "res");

Class<?> c = embedLoader.loadClass("java.lang.Object");
Expand All @@ -514,8 +503,6 @@ public void testIDontHaveIt() throws Exception {
expectThrows(NPE, () -> embedLoader.getResourceAsStream(null));
expectThrows(NPE, () -> embedLoader.resources(null));
expectThrows(NPE, () -> embedLoader.loadClass(null));
} finally {
PrivilegedOperations.closeURLClassLoader(parent);
}
}

Expand All @@ -542,8 +529,7 @@ public void testLoadWithJarDependencies() throws Exception {
JarUtils.createJarWithEntries(outerJar, jarEntries);
URL[] urls = new URL[] { outerJar.toUri().toURL() };

URLClassLoader parent = URLClassLoader.newInstance(urls, EmbeddedImplClassLoaderTests.class.getClassLoader());
try {
try (URLClassLoader parent = loader(urls)) {
EmbeddedImplClassLoader loader = EmbeddedImplClassLoader.getInstance(parent, "blah");
Class<?> c = loader.loadClass("p.Foo");
Object obj = c.getConstructor().newInstance();
Expand All @@ -555,8 +541,6 @@ public void testLoadWithJarDependencies() throws Exception {
expectThrows(CNFE, () -> loader.loadClass("p.Unknown"));
expectThrows(CNFE, () -> loader.loadClass("q.Unknown"));
expectThrows(CNFE, () -> loader.loadClass("r.Unknown"));
} finally {
PrivilegedOperations.closeURLClassLoader(parent);
}
}

Expand All @@ -577,18 +561,20 @@ public void testResourcesWithMultipleJars() throws Exception {
Path outerJar = topLevelDir.resolve("impl.jar");
JarUtils.createJarWithEntriesUTF(outerJar, jarEntries);
URL[] urls = new URL[] { outerJar.toUri().toURL() };
URLClassLoader parent = URLClassLoader.newInstance(urls, EmbeddedImplClassLoaderTests.class.getClassLoader());
try {

try (URLClassLoader parent = loader(urls)) {
EmbeddedImplClassLoader loader = EmbeddedImplClassLoader.getInstance(parent, "blah");
var res = Collections.list(loader.getResources("res.txt"));
assertThat(res, hasSize(3));
List<String> l = res.stream().map(EmbeddedImplClassLoaderTests::urlToString).toList();
assertThat(l, containsInAnyOrder("fooRes", "barRes", "bazRes"));
} finally {
PrivilegedOperations.closeURLClassLoader(parent);
}
}

private static URLClassLoader loader(URL[] urls) {
return URLClassLoader.newInstance(urls, EmbeddedImplClassLoaderTests.class.getClassLoader());
}

@SuppressForbidden(reason = "file urls")
static String urlToString(URL url) {
try {
Expand Down
31 changes: 8 additions & 23 deletions libs/core/src/test/java/org/elasticsearch/core/internal/provider/ProviderLocatorTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@
package org.elasticsearch.core.internal.provider;

import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.PrivilegedOperations;
import org.elasticsearch.test.compiler.InMemoryJavaCompiler;
import org.elasticsearch.test.jar.JarUtils;

import java.lang.module.ModuleDescriptor;
import java.net.MalformedURLException;
import java.net.URL;
import java.net.URLClassLoader;
import java.nio.file.Files;
Expand Down Expand Up @@ -117,12 +117,8 @@ public class FooIntSupplier implements java.util.function.IntSupplier {
Path topLevelDir = createTempDir(getTestName());
Path outerJar = topLevelDir.resolve("impl.jar");
JarUtils.createJarWithEntries(outerJar, jarEntries);
URLClassLoader parent = URLClassLoader.newInstance(
new URL[] { outerJar.toUri().toURL() },
ProviderLocatorTests.class.getClassLoader()
);

try {
try (URLClassLoader parent = loader(outerJar)) {
// test scenario
ProviderLocator<IntSupplier> locator = new ProviderLocator<>("x-foo", IntSupplier.class, parent, "x.foo.impl", Set.of(), true);
IntSupplier impl = locator.get();
Expand All @@ -139,8 +135,6 @@ public class FooIntSupplier implements java.util.function.IntSupplier {
assertThat(md.exports(), containsInAnyOrder(exportsOf("p")));
assertThat(md.opens(), containsInAnyOrder(opensOf("q")));
assertThat(md.packages(), containsInAnyOrder(equalTo("p"), equalTo("q"), equalTo("r")));
} finally {
PrivilegedOperations.closeURLClassLoader(parent);
}
}

Expand Down Expand Up @@ -172,21 +166,15 @@ public class FooLongSupplier implements java.util.function.LongSupplier {
Path topLevelDir = createTempDir(getTestName());
Path outerJar = topLevelDir.resolve("impl.jar");
JarUtils.createJarWithEntries(outerJar, jarEntries);
URLClassLoader parent = URLClassLoader.newInstance(
new URL[] { outerJar.toUri().toURL() },
ProviderLocatorTests.class.getClassLoader()
);

try {
try (URLClassLoader parent = loader(outerJar)) {
// test scenario
ProviderLocator<LongSupplier> locator = new ProviderLocator<>("x-foo", LongSupplier.class, parent, "", Set.of(), false);
LongSupplier impl = locator.get();
assertThat(impl.getAsLong(), is(55L));
assertThat(impl.toString(), equalTo("Hello from FooLongSupplier - non-modular!"));
assertThat(impl.getClass().getName(), equalTo("p.FooLongSupplier"));
assertThat(impl.getClass().getModule().isNamed(), is(false));
} finally {
PrivilegedOperations.closeURLClassLoader(parent);
}
}

Expand Down Expand Up @@ -215,21 +203,18 @@ public class BarIntSupplier implements java.util.function.IntSupplier {
Path pb = Files.createDirectories(barRoot.resolve("pb"));
Files.write(pb.resolve("BarIntSupplier.class"), classToBytes.get("pb.BarIntSupplier"));

URLClassLoader parent = URLClassLoader.newInstance(
new URL[] { topLevelDir.toUri().toURL() },
ProviderLocatorTests.class.getClassLoader()
);

try {
try (URLClassLoader parent = loader(topLevelDir)) {
// test scenario
ProviderLocator<IntSupplier> locator = new ProviderLocator<>("y-bar", IntSupplier.class, parent, "", Set.of(), false);
IntSupplier impl = locator.get();
assertThat(impl.getAsInt(), is(42));
assertThat(impl.toString(), equalTo("Hello from BarIntSupplier - exploded non-modular!"));
assertThat(impl.getClass().getName(), equalTo("pb.BarIntSupplier"));
assertThat(impl.getClass().getModule().isNamed(), is(false));
} finally {
PrivilegedOperations.closeURLClassLoader(parent);
}
}

private static URLClassLoader loader(Path jar) throws MalformedURLException {
return URLClassLoader.newInstance(new URL[] { jar.toUri().toURL() }, ProviderLocatorTests.class.getClassLoader());
}
}
2 changes: 1 addition & 1 deletion modules/lang-painless/spi/src/test/java/org/elasticsearch/painless/WhitelistLoaderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ public void testMissingWhitelistResourceInModule() throws Exception {
JarUtils.createJarWithEntries(jar, jarEntries);

try (var loader = JarUtils.loadJar(jar)) {
Controller controller = JarUtils.loadModule(jar, loader.classloader(), "m");
Controller controller = JarUtils.loadModule(jar, loader, "m");
Module module = controller.layer().findModule("m").orElseThrow();

Class<?> ownerClass = module.getClassLoader().loadClass("p.TestOwner");
Expand Down
9 changes: 4 additions & 5 deletions server/src/test/java/org/elasticsearch/plugins/ExtensionLoaderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
package org.elasticsearch.plugins;

import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.PrivilegedOperations.ClosableURLClassLoader;
import org.elasticsearch.test.compiler.InMemoryJavaCompiler;
import org.elasticsearch.test.jar.JarUtils;

Expand All @@ -35,7 +34,7 @@ public interface TestService {
int getValue();
}

private ClosableURLClassLoader buildProviderJar(Map<String, CharSequence> sources) throws Exception {
private URLClassLoader buildProviderJar(Map<String, CharSequence> sources) throws Exception {
var classToBytes = InMemoryJavaCompiler.compile(sources);

Map<String, byte[]> jarEntries = new HashMap<>();
Expand All @@ -55,7 +54,7 @@ private ClosableURLClassLoader buildProviderJar(Map<String, CharSequence> source
JarUtils.createJarWithEntries(jar, jarEntries);
URL[] urls = new URL[] { jar.toUri().toURL() };

return new ClosableURLClassLoader(URLClassLoader.newInstance(urls, this.getClass().getClassLoader()));
return URLClassLoader.newInstance(urls, this.getClass().getClassLoader());
}

private String defineProvider(String name, int value) {
Expand All @@ -79,7 +78,7 @@ public void testNoProvider() {
public void testOneProvider() throws Exception {
Map<String, CharSequence> sources = Map.of("p.FooService", defineProvider("FooService", 1));
try (var loader = buildProviderJar(sources)) {
TestService service = ExtensionLoader.loadSingleton(ServiceLoader.load(TestService.class, loader.classloader()))
TestService service = ExtensionLoader.loadSingleton(ServiceLoader.load(TestService.class, loader))
.orElseThrow(AssertionError::new);
assertThat(service, not(nullValue()));
assertThat(service.getValue(), equalTo(1));
Expand All @@ -96,7 +95,7 @@ public void testManyProviders() throws Exception {
try (var loader = buildProviderJar(sources)) {
var e = expectThrows(
IllegalStateException.class,
() -> ExtensionLoader.loadSingleton(ServiceLoader.load(TestService.class, loader.classloader()))
() -> ExtensionLoader.loadSingleton(ServiceLoader.load(TestService.class, loader))
);
assertThat(e.getMessage(), containsString("More than one extension found"));
assertThat(e.getMessage(), containsString("TestService"));
Expand Down
6 changes: 1 addition & 5 deletions server/src/test/java/org/elasticsearch/plugins/PluginIntrospectorTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import org.elasticsearch.indices.recovery.plan.ShardSnapshotsService;
import org.elasticsearch.ingest.Processor;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.PrivilegedOperations;
import org.elasticsearch.test.compiler.InMemoryJavaCompiler;
import org.elasticsearch.test.jar.JarUtils;

Expand Down Expand Up @@ -233,11 +232,8 @@ public final class FooPlugin extends q.AbstractFooPlugin { }
JarUtils.createJarWithEntries(jar, jarEntries);
URL[] urls = new URL[] { jar.toUri().toURL() };

URLClassLoader loader = URLClassLoader.newInstance(urls, PluginIntrospectorTests.class.getClassLoader());
try {
try (URLClassLoader loader = URLClassLoader.newInstance(urls, PluginIntrospectorTests.class.getClassLoader())) {
assertThat(pluginIntrospector.interfaces(loader.loadClass("r.FooPlugin")), contains("ActionPlugin"));
} finally {
PrivilegedOperations.closeURLClassLoader(loader);
}
}

Expand Down
5 changes: 2 additions & 3 deletions server/src/test/java/org/elasticsearch/plugins/PluginsLoaderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import org.elasticsearch.nativeaccess.NativeAccessUtil;
import org.elasticsearch.plugin.analysis.CharFilterFactory;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.PrivilegedOperations;
import org.elasticsearch.test.compiler.InMemoryJavaCompiler;
import org.elasticsearch.test.jar.JarUtils;

Expand Down Expand Up @@ -351,13 +350,13 @@ static void closePluginLoaders(PluginsLoader pluginsLoader) {
pluginsLoader.pluginLayers().forEach(lp -> {
if (lp.pluginClassLoader() instanceof URLClassLoader urlClassLoader) {
try {
PrivilegedOperations.closeURLClassLoader(urlClassLoader);
urlClassLoader.close();
} catch (IOException unexpected) {
throw new UncheckedIOException(unexpected);
}
} else if (lp.pluginClassLoader() instanceof UberModuleClassLoader loader) {
try {
PrivilegedOperations.closeURLClassLoader(loader.getInternalLoader());
loader.getInternalLoader().close();
} catch (Exception e) {
throw new RuntimeException(e);
}
Expand Down
16 changes: 7 additions & 9 deletions server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import org.elasticsearch.plugins.spi.BarTestService;
import org.elasticsearch.plugins.spi.TestService;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.PrivilegedOperations;
import org.elasticsearch.test.compiler.InMemoryJavaCompiler;
import org.elasticsearch.test.jar.JarUtils;

Expand Down Expand Up @@ -671,9 +670,11 @@ public String name() {
}

public void testLoadServiceProviders() throws Exception {
URLClassLoader fakeClassLoader = buildTestProviderPlugin("integer");
URLClassLoader fakeClassLoader1 = buildTestProviderPlugin("string");
try {

try (
URLClassLoader fakeClassLoader = buildTestProviderPlugin("integer");
URLClassLoader fakeClassLoader1 = buildTestProviderPlugin("string")
) {
@SuppressWarnings("unchecked")
Class<? extends Plugin> fakePluginClass = (Class<? extends Plugin>) fakeClassLoader.loadClass("r.FooPlugin");
@SuppressWarnings("unchecked")
Expand All @@ -699,9 +700,6 @@ public void testLoadServiceProviders() throws Exception {
providers = service.loadServiceProviders(TestService.class);

assertEquals(0, providers.size());
} finally {
PrivilegedOperations.closeURLClassLoader(fakeClassLoader);
PrivilegedOperations.closeURLClassLoader(fakeClassLoader1);
}
}

Expand Down Expand Up @@ -877,13 +875,13 @@ static void closePluginLoaders(PluginsService pluginService) {
for (var lp : pluginService.plugins()) {
if (lp.classLoader() instanceof URLClassLoader urlClassLoader) {
try {
PrivilegedOperations.closeURLClassLoader(urlClassLoader);
urlClassLoader.close();
} catch (IOException unexpected) {
throw new UncheckedIOException(unexpected);
}
} else if (lp.classLoader() instanceof UberModuleClassLoader loader) {
try {
PrivilegedOperations.closeURLClassLoader(loader.getInternalLoader());
loader.getInternalLoader().close();
} catch (Exception e) {
throw new RuntimeException(e);
}
Expand Down
9 changes: 0 additions & 9 deletions test/framework/src/main/java/org/elasticsearch/bootstrap/BootstrapForTesting.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,9 @@
import org.elasticsearch.core.Booleans;
import org.elasticsearch.core.PathUtils;
import org.elasticsearch.jdk.JarHell;
import org.elasticsearch.test.PrivilegedOperations;
import org.elasticsearch.test.mockito.SecureMockMaker;

import java.io.IOException;
import java.lang.invoke.MethodHandles;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Objects;
Expand Down Expand Up @@ -75,13 +73,6 @@ public class BootstrapForTesting {
// init mockito
SecureMockMaker.init();

// init the privileged operation
try {
MethodHandles.publicLookup().ensureInitialized(PrivilegedOperations.class);
} catch (IllegalAccessException unexpected) {
throw new AssertionError(unexpected);
}

// Log ifconfig output before SecurityManager is installed
IfConfig.logIfNecessary();
}
Expand Down
Loading