This is an automated email from the ASF dual-hosted git repository.
hansva pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/hop.git
The following commit(s) were added to refs/heads/main by this push:
new 7525d30ef8 harden VFS namespace split, fixes #8215 (#8231)
7525d30ef8 is described below
commit 7525d30ef8e818d9d306bca599531286c7ba1dfa
Author: Hans Van Akelyen <[email protected]>
AuthorDate: Wed Sep 2 11:47:35 2026 +0200
harden VFS namespace split, fixes #8215 (#8231)
---
.../main/java/org/apache/hop/core/vfs/HopVfs.java | 41 ++++--
.../org/apache/hop/core/vfs/HopVfsNamespace.java | 8 ++
.../org/apache/hop/core/vfs/HopVfsNamespaces.java | 13 ++
.../hop/core/vfs/HopVfsNamespaceScopingTest.java | 140 +++++++++++++++++++++
.../java/org/apache/hop/pipeline/Pipeline.java | 4 +-
.../java/org/apache/hop/workflow/Workflow.java | 4 +-
6 files changed, 199 insertions(+), 11 deletions(-)
diff --git a/core/src/main/java/org/apache/hop/core/vfs/HopVfs.java
b/core/src/main/java/org/apache/hop/core/vfs/HopVfs.java
index 2928896d9f..4787e127d3 100644
--- a/core/src/main/java/org/apache/hop/core/vfs/HopVfs.java
+++ b/core/src/main/java/org/apache/hop/core/vfs/HopVfs.java
@@ -105,7 +105,7 @@ public class HopVfs {
// them, so start over. Nothing registered yet means we can keep the
manager as it is: phase 2
// simply runs with these variables the next time around.
//
- if (namedProvidersRegistered) {
+ if (namedProvidersRegistered && !HopVfsNamespaces.isIsolated()) {
reset();
}
}
@@ -748,7 +748,9 @@ public class HopVfs {
*/
public static boolean startsWithScheme(String vfsFileName, IVariables
variables) {
bootstrapWith(variables);
- return startsWithScheme(vfsFileName);
+ // The schemes that apply are the ones of the namespace these variables
resolve files in: the
+ // named connections of an export, or of a Hop Web session, are not on the
process manager.
+ return startsWithScheme(vfsFileName, getFileSystemManager(variables));
}
/**
@@ -759,8 +761,13 @@ public class HopVfs {
* @return boolean
*/
public static boolean startsWithScheme(String vfsFileName) {
- DefaultFileSystemManager fsManager = getFileSystemManager();
+ // Nothing to go on but the thread: the namespace of the execution running
on it, if any.
+ HopVfsNamespace namespace = HopVfsNamespaces.getCurrent();
+ return startsWithScheme(
+ vfsFileName, namespace == null ? getFileSystemManager() :
namespace.getFileSystemManager());
+ }
+ private static boolean startsWithScheme(String vfsFileName,
DefaultFileSystemManager fsManager) {
boolean found = false;
String[] schemes = fsManager.getSchemes();
for (String scheme : schemes) {
@@ -843,10 +850,24 @@ public class HopVfs {
}
/**
- * Drop the file system manager so it's rebuilt, providers of the named VFS
connections included,
- * the next time it's used. The bootstrap variables are kept: use {@link
- * #setBootstrapVariables(IVariables)} to change those.
+ * Let go of the file systems nobody is using any more in the namespace
these variables resolve
+ * files in.
+ *
+ * <p>Use this rather than {@link #freeUnusedResources()} at the end of an
execution: an execution
+ * carrying its own metadata resolved its files in a namespace of its own,
and those are the file
+ * systems it is done with. The process wide manager belongs to whoever else
is in this JVM.
+ *
+ * @param variables the variables of the execution that just ended
*/
+ public static void freeUnusedResources(IVariables variables) {
+ HopVfsNamespace namespace = HopVfsNamespaces.resolve(variables);
+ if (namespace != null) {
+ namespace.freeUnusedResources();
+ return;
+ }
+ freeUnusedResources();
+ }
+
/**
* Read the named VFS connections again for whoever these variables belong
to, after one of them
* was added or changed.
@@ -871,7 +892,13 @@ public class HopVfs {
"Error re-reading the named VFS connections of this caller. They are
unchanged.", e);
return;
}
- // Nothing of their own to refresh: what they resolve files in is the
process wide manager.
+ if (HopVfsNamespaces.isIsolated()) {
+ LogChannel.GENERAL.logDebug(
+ "No VFS namespace to re-read the named connections for. The process
wide file system "
+ + "manager is left alone: other sessions are using it.");
+ return;
+ }
+ // What they resolve files in is the process wide manager.
reset();
}
diff --git a/core/src/main/java/org/apache/hop/core/vfs/HopVfsNamespace.java
b/core/src/main/java/org/apache/hop/core/vfs/HopVfsNamespace.java
index eebbaccbaa..29ef06748c 100644
--- a/core/src/main/java/org/apache/hop/core/vfs/HopVfsNamespace.java
+++ b/core/src/main/java/org/apache/hop/core/vfs/HopVfsNamespace.java
@@ -123,6 +123,14 @@ public class HopVfsNamespace implements AutoCloseable {
return description;
}
+ /**
+ * Close the communication links of the file systems in this namespace that
nothing is using any
+ * more. What an execution does when it ends, for its own namespace and
nobody else's.
+ */
+ public void freeUnusedResources() {
+ fileSystemManager.freeUnusedResources();
+ }
+
int retain() {
return ++useCount;
}
diff --git a/core/src/main/java/org/apache/hop/core/vfs/HopVfsNamespaces.java
b/core/src/main/java/org/apache/hop/core/vfs/HopVfsNamespaces.java
index 66d9dc1542..71e3adae1e 100644
--- a/core/src/main/java/org/apache/hop/core/vfs/HopVfsNamespaces.java
+++ b/core/src/main/java/org/apache/hop/core/vfs/HopVfsNamespaces.java
@@ -80,6 +80,19 @@ public class HopVfsNamespaces {
/** Set when the runtime serves several tenants at once, so nobody shares
the process manager. */
private static boolean isolateEverything;
+ /**
+ * Does this JVM serve several tenants at once - Hop Web, with a session
scope installed?
+ *
+ * <p>What it really answers is whether anything process wide may be thrown
away on behalf of one
+ * of them. It may not: every tenant has a namespace of its own and the ones
that are not asking
+ * are in the middle of something.
+ *
+ * @return true when a runtime installed a scope of its own with {@link
#setScope(IHopScope)}
+ */
+ public static synchronized boolean isIsolated() {
+ return isolateEverything;
+ }
+
private HopVfsNamespaces() {
// Utility class
}
diff --git
a/core/src/test/java/org/apache/hop/core/vfs/HopVfsNamespaceScopingTest.java
b/core/src/test/java/org/apache/hop/core/vfs/HopVfsNamespaceScopingTest.java
index 7748cd648e..2bda4bbed2 100644
--- a/core/src/test/java/org/apache/hop/core/vfs/HopVfsNamespaceScopingTest.java
+++ b/core/src/test/java/org/apache/hop/core/vfs/HopVfsNamespaceScopingTest.java
@@ -17,6 +17,7 @@
package org.apache.hop.core.vfs;
+import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNotSame;
import static org.junit.jupiter.api.Assertions.assertNull;
@@ -25,9 +26,11 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;
import org.apache.commons.vfs2.impl.DefaultFileSystemManager;
+import org.apache.commons.vfs2.provider.ram.RamFileProvider;
import org.apache.hop.core.logging.HopLogStore;
import org.apache.hop.core.scope.IHopScope;
import org.apache.hop.core.variables.Variables;
+import org.apache.hop.metadata.api.IHopMetadataProvider;
import org.apache.hop.metadata.serializer.multi.MultiMetadataProvider;
import org.apache.hop.metadata.util.HopMetadataInstance;
import org.junit.jupiter.api.AfterEach;
@@ -71,6 +74,16 @@ class HopVfsNamespaceScopingTest {
return perSession;
}
+ /** The variables of something running against this metadata, the way an
execution's are. */
+ private Variables variablesOf(IHopMetadataProvider provider) {
+ return new Variables() {
+ @Override
+ public IHopMetadataProvider getMetadataProvider() {
+ return provider;
+ }
+ };
+ }
+
/** Build the process wide manager the way loading a project does, from this
metadata. */
private void bootstrapProcessManagerFrom(MultiMetadataProvider provider) {
HopMetadataInstance.setMetadataProvider(provider);
@@ -196,6 +209,133 @@ class HopVfsNamespaceScopingTest {
}
}
+ @Test
+ @DisplayName("Enabling a project in one session leaves the namespaces of the
others alone")
+ void enablingAProjectDoesNotResetOtherSessions() {
+ // Enabling a project hands HopVfs the variables to find that project's
named connections with
+ // (ProjectsUtil.enableProject). In Hop Web that happens once per session,
and it must not take
+ // the file system out from under the sessions already running. See issue
#8215.
+ MultiMetadataProvider sessionA = mock(MultiMetadataProvider.class);
+ MultiMetadataProvider sessionB = mock(MultiMetadataProvider.class);
+
+ installSessionScopes();
+ bootstrapProcessManagerFrom(sessionA);
+
+ HopVfsNamespace namespaceA = HopVfsNamespaces.acquire(new Variables(),
sessionA, "session A");
+ HopVfsNamespace namespaceB = HopVfsNamespaces.acquire(new Variables(),
sessionB, "session B");
+ try {
+ DefaultFileSystemManager managerOfA = namespaceA.getFileSystemManager();
+
+ // Session B switches project: new variables, pointing at the metadata
of the new project.
+ HopVfs.setBootstrapVariables(Variables.getADefaultVariableSpace());
+
+ assertSame(2, HopVfsNamespaces.size(), "Neither session should have lost
its namespace");
+ assertSame(
+ managerOfA,
+ namespaceA.getFileSystemManager(),
+ "Session A was in the middle of something and must not have lost its
file system");
+ assertTrue(
+ managerOfA.hasProvider("file"),
+ "The file system manager of session A must still be open");
+ } finally {
+ HopVfsNamespaces.release(sessionA);
+ HopVfsNamespaces.release(sessionB);
+ }
+ }
+
+ @Test
+ @DisplayName("A session with no namespace of its own still does not reset
the shared manager")
+ void refreshWithoutANamespaceLeavesTheOthersAlone() {
+ // Saving a VFS connection from a session that has not opened a project
yet: there is no
+ // namespace to re-read the connections for, and emptying the process wide
manager instead
+ // would take the sessions that do have one down with it. See issue #8215.
+ MultiMetadataProvider sessionA = mock(MultiMetadataProvider.class);
+
+ installSessionScopes();
+ bootstrapProcessManagerFrom(sessionA);
+
+ HopVfsNamespace namespaceA = HopVfsNamespaces.acquire(new Variables(),
sessionA, "session A");
+ try {
+ DefaultFileSystemManager managerOfA = namespaceA.getFileSystemManager();
+
+ HopVfs.refresh(new Variables());
+
+ assertSame(1, HopVfsNamespaces.size(), "Session A should still have its
namespace");
+ assertSame(managerOfA, namespaceA.getFileSystemManager());
+ assertTrue(managerOfA.hasProvider("file"), "The manager of session A
must still be open");
+ } finally {
+ HopVfsNamespaces.release(sessionA);
+ }
+ }
+
+ @Test
+ @DisplayName("An execution frees the resources of its own namespace, not of
the shared manager")
+ void freeingResourcesStaysInsideTheCallersNamespace() {
+ // What Pipeline and Workflow do when they end. The file systems they are
done with are the
+ // ones in their own namespace: the process wide manager belongs to
whoever else is in the JVM.
+ MultiMetadataProvider processMetadata = mock(MultiMetadataProvider.class);
+ bootstrapProcessManagerFrom(processMetadata);
+
+ MultiMetadataProvider exportedMetadata = mock(MultiMetadataProvider.class);
+ HopVfsNamespace namespace =
+ HopVfsNamespaces.acquire(new Variables(), exportedMetadata, "exported
pipeline");
+ try {
+ CountingProvider ofTheExport = new CountingProvider();
+ CountingProvider ofEveryoneElse = new CountingProvider();
+ namespace.getFileSystemManager().addProvider("exportdrop", ofTheExport);
+ HopVfs.getFileSystemManager().addProvider("shareddrop", ofEveryoneElse);
+
+ HopVfs.freeUnusedResources(variablesOf(exportedMetadata));
+
+ assertSame(
+ 1, ofTheExport.timesFreed, "The export is done with the files of its
own namespace");
+ assertSame(0, ofEveryoneElse.timesFreed, "Nobody else's file systems are
its business");
+ } catch (Exception e) {
+ throw new AssertionError("Unable to register a provider", e);
+ } finally {
+ HopVfsNamespaces.release(exportedMetadata);
+ }
+ }
+
+ /** A provider that says whether the manager it sits on was asked to free
its resources. */
+ private static class CountingProvider extends RamFileProvider {
+ private int timesFreed;
+
+ @Override
+ public void freeUnusedResources() {
+ timesFreed++;
+ super.freeUnusedResources();
+ }
+ }
+
+ @Test
+ @DisplayName("A scheme is looked for in the namespace of the caller, not on
the shared manager")
+ void startsWithSchemeAsksTheCallersNamespace() {
+ // The name of a named VFS connection is a scheme, and the connections of
an export - or of a
+ // Hop Web session - live in a namespace of their own. Anything deciding
whether a filename
+ // carries a scheme has to ask the manager that filename will be resolved
on.
+ MultiMetadataProvider processMetadata = mock(MultiMetadataProvider.class);
+ bootstrapProcessManagerFrom(processMetadata);
+
+ MultiMetadataProvider exportedMetadata = mock(MultiMetadataProvider.class);
+ HopVfsNamespace namespace =
+ HopVfsNamespaces.acquire(new Variables(), exportedMetadata, "exported
pipeline");
+ try {
+ namespace.getFileSystemManager().addProvider("salesdrop", new
RamFileProvider());
+
+ assertTrue(
+ HopVfs.startsWithScheme("salesdrop://in/orders.csv",
variablesOf(exportedMetadata)),
+ "The connection of the export is a scheme its own namespace knows");
+ assertFalse(
+ HopVfs.startsWithScheme("salesdrop://in/orders.csv", new
Variables()),
+ "Nobody else sees the connections of this export");
+ } catch (Exception e) {
+ throw new AssertionError("Unable to register a provider on the
namespace", e);
+ } finally {
+ HopVfsNamespaces.release(exportedMetadata);
+ }
+ }
+
@Test
@DisplayName("A scope that answers per session never falls back to what a
pooled thread left")
void aSessionNeverInheritsAnotherSessionsNamespace() throws Exception {
diff --git a/engine/src/main/java/org/apache/hop/pipeline/Pipeline.java
b/engine/src/main/java/org/apache/hop/pipeline/Pipeline.java
index cd8ba66612..609e6a16f6 100644
--- a/engine/src/main/java/org/apache/hop/pipeline/Pipeline.java
+++ b/engine/src/main/java/org/apache/hop/pipeline/Pipeline.java
@@ -1467,8 +1467,8 @@ public abstract class Pipeline
// Safe here: all transform threads have finished before this
listener runs.
cleanupRowSets();
- // release unused vfs connections
- HopVfs.freeUnusedResources();
+ // release unused vfs connections, of the namespace this pipeline
resolved its files in
+ HopVfs.freeUnusedResources(this);
};
// This should always be done first so that the other listeners achieve a
clean state to start
// from (setFinished and
diff --git a/engine/src/main/java/org/apache/hop/workflow/Workflow.java
b/engine/src/main/java/org/apache/hop/workflow/Workflow.java
index 07282b78a3..7785afdbe3 100644
--- a/engine/src/main/java/org/apache/hop/workflow/Workflow.java
+++ b/engine/src/main/java/org/apache/hop/workflow/Workflow.java
@@ -358,8 +358,8 @@ public abstract class Workflow extends Variables
log.logBasic(BaseMessages.getString(PKG, CONST_WORKFLOW_FINISHED));
fireExecutionFinishedListeners();
- // release unused vfs connections
- HopVfs.freeUnusedResources();
+ // release unused vfs connections, of the namespace this workflow
resolved its files in
+ HopVfs.freeUnusedResources(this);
} catch (HopException e) {
result.setNrErrors(1);
