From 97f7c12830686ba7ea0b9b284a91d1989ff8a261 Mon Sep 17 00:00:00 2001 From: dae won Date: Tue, 28 Jul 2026 12:29:27 +0900 Subject: [PATCH 1/2] [ZEPPELIN-6462] Close interpreter-setting.json streams with try-with-resources Both registration paths handed a freshly opened InputStream to Gson without closing it, and Gson does not close a reader passed to it. The descriptor leaked on the normal path and on the parse-failure path alike, and registration re-runs on every interpreter install. --- .../InterpreterSettingManager.java | 33 ++++++-- .../InterpreterSettingManagerTest.java | 84 +++++++++++++++++++ 2 files changed, 108 insertions(+), 9 deletions(-) diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java b/zeppelin-server/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java index e2959382206..5ecfac5cb84 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java @@ -475,8 +475,9 @@ public ApplicationEventListener getAppEventListener() { return appEventListener; } - private boolean registerInterpreterFromResource(ClassLoader cl, String interpreterDir, - String interpreterJson, boolean override) throws IOException { + boolean registerInterpreterFromResource(ClassLoader cl, String interpreterDir, + String interpreterJson, boolean override) + throws IOException { URL[] urls = recursiveBuildLibList(new File(interpreterDir)); ClassLoader tempClassLoader = new URLClassLoader(urls, null); @@ -486,26 +487,40 @@ private boolean registerInterpreterFromResource(ClassLoader cl, String interpret } LOGGER.debug("Reading interpreter-setting.json from {} as Resource", url); - List registeredInterpreterList = - getInterpreterListFromJson(url.openStream()); - registerInterpreterSetting(registeredInterpreterList, interpreterDir, override); + try (InputStream stream = openStream(url)) { + List registeredInterpreterList = getInterpreterListFromJson(stream); + registerInterpreterSetting(registeredInterpreterList, interpreterDir, override); + } return true; } - private boolean registerInterpreterFromPath(String interpreterDir, String interpreterJson, + boolean registerInterpreterFromPath(String interpreterDir, String interpreterJson, boolean override) throws IOException { Path interpreterJsonPath = Paths.get(interpreterDir, interpreterJson); if (Files.exists(interpreterJsonPath)) { LOGGER.debug("Reading interpreter-setting.json from file {}", interpreterJsonPath); - List registeredInterpreterList = - getInterpreterListFromJson(new FileInputStream(interpreterJsonPath.toFile())); - registerInterpreterSetting(registeredInterpreterList, interpreterDir, override); + try (InputStream stream = openStream(interpreterJsonPath.toFile())) { + List registeredInterpreterList = getInterpreterListFromJson(stream); + registerInterpreterSetting(registeredInterpreterList, interpreterDir, override); + } return true; } return false; } + // allows a tracked stream to be injected in unit tests. + @VisibleForTesting + InputStream openStream(URL url) throws IOException { + return url.openStream(); + } + + // allows a tracked stream to be injected in unit tests. + @VisibleForTesting + InputStream openStream(File file) throws IOException { + return new FileInputStream(file); + } + private List getInterpreterListFromJson(InputStream stream) { Type registeredInterpreterListType = new TypeToken>() { }.getType(); diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/interpreter/InterpreterSettingManagerTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/interpreter/InterpreterSettingManagerTest.java index 54d3ffaeb08..cdb6029d70f 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/interpreter/InterpreterSettingManagerTest.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/interpreter/InterpreterSettingManagerTest.java @@ -18,6 +18,8 @@ package org.apache.zeppelin.interpreter; +import com.google.gson.JsonSyntaxException; + import org.apache.zeppelin.conf.ZeppelinConfiguration; import org.apache.zeppelin.dep.Dependency; import org.apache.zeppelin.display.AngularObjectRegistryListener; @@ -26,24 +28,47 @@ import org.apache.zeppelin.user.AuthenticationInfo; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import org.eclipse.aether.RepositoryException; import org.eclipse.aether.repository.RemoteRepository; +import java.io.ByteArrayInputStream; +import java.io.File; import java.io.IOException; +import java.io.InputStream; +import java.net.URL; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.jar.JarEntry; +import java.util.jar.JarOutputStream; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; class InterpreterSettingManagerTest extends AbstractInterpreterTest { + private static final String INTERPRETER_SETTING_JSON = "interpreter-setting.json"; + + private static final String STREAM_TEST_SETTING = + "[{\"group\": \"stream_close_test\"," + + " \"name\": \"stream_close_test\"," + + " \"className\": \"org.apache.zeppelin.interpreter.EchoInterpreter\"," + + " \"properties\": {}}]"; + private String note1Id; private String note2Id; private String note3Id; @@ -342,4 +367,63 @@ void testInterpreterIncludeExcludeTogether() throws Exception { System.clearProperty(ZeppelinConfiguration.ConfVars.ZEPPELIN_INTERPRETER_EXCLUDES.getVarName()); } } + + @Test + void testRegisterInterpreterFromPathClosesStream(@TempDir Path interpreterDir) + throws IOException { + Files.write(interpreterDir.resolve(INTERPRETER_SETTING_JSON), + STREAM_TEST_SETTING.getBytes(StandardCharsets.UTF_8)); + + InterpreterSettingManager spyManager = spy(interpreterSettingManager); + InputStream stream = spy( + new ByteArrayInputStream(STREAM_TEST_SETTING.getBytes(StandardCharsets.UTF_8))); + doReturn(stream).when(spyManager).openStream(any(File.class)); + + assertTrue(spyManager.registerInterpreterFromPath( + interpreterDir.toString(), INTERPRETER_SETTING_JSON, false)); + + verify(stream).close(); + } + + @Test + void testRegisterInterpreterFromResourceClosesStream(@TempDir Path interpreterDir) + throws IOException { + // registerInterpreterFromResource builds a URLClassLoader over every file in the + // interpreter directory, so the setting has to be reachable from a jar entry. + writeSettingJar(interpreterDir.resolve("stream-close-test.jar")); + + InterpreterSettingManager spyManager = spy(interpreterSettingManager); + InputStream stream = spy( + new ByteArrayInputStream(STREAM_TEST_SETTING.getBytes(StandardCharsets.UTF_8))); + doReturn(stream).when(spyManager).openStream(any(URL.class)); + + assertTrue(spyManager.registerInterpreterFromResource( + getClass().getClassLoader(), interpreterDir.toString(), INTERPRETER_SETTING_JSON, false)); + + verify(stream).close(); + } + + @Test + void testStreamIsClosedWhenParsingFails(@TempDir Path interpreterDir) throws IOException { + Files.write(interpreterDir.resolve(INTERPRETER_SETTING_JSON), + "not json".getBytes(StandardCharsets.UTF_8)); + + InterpreterSettingManager spyManager = spy(interpreterSettingManager); + InputStream stream = spy( + new ByteArrayInputStream("not json".getBytes(StandardCharsets.UTF_8))); + doReturn(stream).when(spyManager).openStream(any(File.class)); + + assertThrows(JsonSyntaxException.class, () -> spyManager.registerInterpreterFromPath( + interpreterDir.toString(), INTERPRETER_SETTING_JSON, false)); + + verify(stream).close(); + } + + private static void writeSettingJar(Path jarPath) throws IOException { + try (JarOutputStream jar = new JarOutputStream(Files.newOutputStream(jarPath))) { + jar.putNextEntry(new JarEntry(INTERPRETER_SETTING_JSON)); + jar.write(STREAM_TEST_SETTING.getBytes(StandardCharsets.UTF_8)); + jar.closeEntry(); + } + } } From 42117ad984671c89f3c9dee02a55193d717e1650 Mon Sep 17 00:00:00 2001 From: dae won Date: Tue, 4 Aug 2026 16:04:51 +0900 Subject: [PATCH 2/2] [ZEPPELIN-6462] Remove test-only openStream seams and their tests --- .../InterpreterSettingManager.java | 23 ++--- .../InterpreterSettingManagerTest.java | 84 ------------------- 2 files changed, 5 insertions(+), 102 deletions(-) diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java b/zeppelin-server/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java index 5ecfac5cb84..f6086f4d06c 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java @@ -475,9 +475,8 @@ public ApplicationEventListener getAppEventListener() { return appEventListener; } - boolean registerInterpreterFromResource(ClassLoader cl, String interpreterDir, - String interpreterJson, boolean override) - throws IOException { + private boolean registerInterpreterFromResource(ClassLoader cl, String interpreterDir, + String interpreterJson, boolean override) throws IOException { URL[] urls = recursiveBuildLibList(new File(interpreterDir)); ClassLoader tempClassLoader = new URLClassLoader(urls, null); @@ -487,20 +486,20 @@ boolean registerInterpreterFromResource(ClassLoader cl, String interpreterDir, } LOGGER.debug("Reading interpreter-setting.json from {} as Resource", url); - try (InputStream stream = openStream(url)) { + try (InputStream stream = url.openStream()) { List registeredInterpreterList = getInterpreterListFromJson(stream); registerInterpreterSetting(registeredInterpreterList, interpreterDir, override); } return true; } - boolean registerInterpreterFromPath(String interpreterDir, String interpreterJson, + private boolean registerInterpreterFromPath(String interpreterDir, String interpreterJson, boolean override) throws IOException { Path interpreterJsonPath = Paths.get(interpreterDir, interpreterJson); if (Files.exists(interpreterJsonPath)) { LOGGER.debug("Reading interpreter-setting.json from file {}", interpreterJsonPath); - try (InputStream stream = openStream(interpreterJsonPath.toFile())) { + try (InputStream stream = new FileInputStream(interpreterJsonPath.toFile())) { List registeredInterpreterList = getInterpreterListFromJson(stream); registerInterpreterSetting(registeredInterpreterList, interpreterDir, override); } @@ -509,18 +508,6 @@ boolean registerInterpreterFromPath(String interpreterDir, String interpreterJso return false; } - // allows a tracked stream to be injected in unit tests. - @VisibleForTesting - InputStream openStream(URL url) throws IOException { - return url.openStream(); - } - - // allows a tracked stream to be injected in unit tests. - @VisibleForTesting - InputStream openStream(File file) throws IOException { - return new FileInputStream(file); - } - private List getInterpreterListFromJson(InputStream stream) { Type registeredInterpreterListType = new TypeToken>() { }.getType(); diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/interpreter/InterpreterSettingManagerTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/interpreter/InterpreterSettingManagerTest.java index cdb6029d70f..54d3ffaeb08 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/interpreter/InterpreterSettingManagerTest.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/interpreter/InterpreterSettingManagerTest.java @@ -18,8 +18,6 @@ package org.apache.zeppelin.interpreter; -import com.google.gson.JsonSyntaxException; - import org.apache.zeppelin.conf.ZeppelinConfiguration; import org.apache.zeppelin.dep.Dependency; import org.apache.zeppelin.display.AngularObjectRegistryListener; @@ -28,47 +26,24 @@ import org.apache.zeppelin.user.AuthenticationInfo; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.io.TempDir; import org.eclipse.aether.RepositoryException; import org.eclipse.aether.repository.RemoteRepository; -import java.io.ByteArrayInputStream; -import java.io.File; import java.io.IOException; -import java.io.InputStream; -import java.net.URL; -import java.nio.charset.StandardCharsets; -import java.nio.file.Files; -import java.nio.file.Path; import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.jar.JarEntry; -import java.util.jar.JarOutputStream; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.verify; class InterpreterSettingManagerTest extends AbstractInterpreterTest { - private static final String INTERPRETER_SETTING_JSON = "interpreter-setting.json"; - - private static final String STREAM_TEST_SETTING = - "[{\"group\": \"stream_close_test\"," - + " \"name\": \"stream_close_test\"," - + " \"className\": \"org.apache.zeppelin.interpreter.EchoInterpreter\"," - + " \"properties\": {}}]"; - private String note1Id; private String note2Id; private String note3Id; @@ -367,63 +342,4 @@ void testInterpreterIncludeExcludeTogether() throws Exception { System.clearProperty(ZeppelinConfiguration.ConfVars.ZEPPELIN_INTERPRETER_EXCLUDES.getVarName()); } } - - @Test - void testRegisterInterpreterFromPathClosesStream(@TempDir Path interpreterDir) - throws IOException { - Files.write(interpreterDir.resolve(INTERPRETER_SETTING_JSON), - STREAM_TEST_SETTING.getBytes(StandardCharsets.UTF_8)); - - InterpreterSettingManager spyManager = spy(interpreterSettingManager); - InputStream stream = spy( - new ByteArrayInputStream(STREAM_TEST_SETTING.getBytes(StandardCharsets.UTF_8))); - doReturn(stream).when(spyManager).openStream(any(File.class)); - - assertTrue(spyManager.registerInterpreterFromPath( - interpreterDir.toString(), INTERPRETER_SETTING_JSON, false)); - - verify(stream).close(); - } - - @Test - void testRegisterInterpreterFromResourceClosesStream(@TempDir Path interpreterDir) - throws IOException { - // registerInterpreterFromResource builds a URLClassLoader over every file in the - // interpreter directory, so the setting has to be reachable from a jar entry. - writeSettingJar(interpreterDir.resolve("stream-close-test.jar")); - - InterpreterSettingManager spyManager = spy(interpreterSettingManager); - InputStream stream = spy( - new ByteArrayInputStream(STREAM_TEST_SETTING.getBytes(StandardCharsets.UTF_8))); - doReturn(stream).when(spyManager).openStream(any(URL.class)); - - assertTrue(spyManager.registerInterpreterFromResource( - getClass().getClassLoader(), interpreterDir.toString(), INTERPRETER_SETTING_JSON, false)); - - verify(stream).close(); - } - - @Test - void testStreamIsClosedWhenParsingFails(@TempDir Path interpreterDir) throws IOException { - Files.write(interpreterDir.resolve(INTERPRETER_SETTING_JSON), - "not json".getBytes(StandardCharsets.UTF_8)); - - InterpreterSettingManager spyManager = spy(interpreterSettingManager); - InputStream stream = spy( - new ByteArrayInputStream("not json".getBytes(StandardCharsets.UTF_8))); - doReturn(stream).when(spyManager).openStream(any(File.class)); - - assertThrows(JsonSyntaxException.class, () -> spyManager.registerInterpreterFromPath( - interpreterDir.toString(), INTERPRETER_SETTING_JSON, false)); - - verify(stream).close(); - } - - private static void writeSettingJar(Path jarPath) throws IOException { - try (JarOutputStream jar = new JarOutputStream(Files.newOutputStream(jarPath))) { - jar.putNextEntry(new JarEntry(INTERPRETER_SETTING_JSON)); - jar.write(STREAM_TEST_SETTING.getBytes(StandardCharsets.UTF_8)); - jar.closeEntry(); - } - } }