Closing contexts in tests, cleanups

Signed-off-by: David Matějček <david.matejcek@omnifish.ee>

appserver/common/glassfish-naming/src/main/java/com/sun/enterprise/naming/impl/GlassfishNamingManagerImpl.java

@@ -475,7 +475,7 @@ private void bindIntermediateContexts(JavaNamespace namespace, SimpleJndiName jn
             sb.append('/').append(tok);
             final SimpleJndiName nsJndiName = new SimpleJndiName(sb.toString());
             if (namespace.get(nsJndiName) == null) {
-                namespace.put(nsJndiName, new JavaURLContext(nsJndiName, null));
+                namespace.put(nsJndiName, new JavaURLContext(nsJndiName));
             }
         }
     }
@@ -527,7 +527,7 @@ public void unbindModuleObject(String appName, String moduleName, SimpleJndiName
     @Override
     public Context restoreJavaCompEnvContext(SimpleJndiName contextName) throws NamingException {
         if (contextName.hasJavaPrefix()) {
-            return new JavaURLContext(contextName, null);
+            return new JavaURLContext(contextName);
         }
         throw new NamingException("Invalid context name [" + contextName + "]. Name must start with java:");
     }
@@ -583,14 +583,14 @@ private <T> T lookup(String componentId, SimpleJndiName name, Context ctx) throw
             // of a replaced java:comp, create a new equivalent javaURLContext
             // and return that.
             if (replaceName) {
-                obj = new JavaURLContext(name, null);
+                obj = new JavaURLContext(name);
             }
 
             if (obj instanceof JavaURLContext) {
                 if (ctx instanceof SerialContext) {
                     return (T) new JavaURLContext((JavaURLContext) obj, (SerialContext) ctx);
                 }
-                return (T) new JavaURLContext((JavaURLContext) obj, null);
+                return (T) new JavaURLContext((JavaURLContext) obj);
             }
         }
         return (T) obj;

appserver/common/glassfish-naming/src/main/java/com/sun/enterprise/naming/impl/JavaURLContext.java

@@ -60,6 +60,7 @@ public final class JavaURLContext implements Context, Cloneable {
 
     private final SimpleJndiName myName;
     private final Hashtable<Object, Object> myEnv;
+    // FIXME: Should not be here, causes cyclic dependency directly between these two classes.
     private final SerialContext serialContext;
 
 
@@ -71,47 +72,43 @@ static void setNamingManager(GlassfishNamingManagerImpl mgr) {
      * Create a context with the specified environment.
      */
     public JavaURLContext(SimpleJndiName name) {
-        this.myEnv = getMyEnv(null);
         this.myName = Objects.requireNonNull(name, "name");
+        this.myEnv = getMyEnv(null);
         this.serialContext = null;
     }
 
 
     /**
-     * Create a context with the specified environment.
+     * Create a context with the specified name+environment.
      */
-    public JavaURLContext(Hashtable<Object, Object> environment) {
+    public JavaURLContext(SimpleJndiName name, Hashtable<Object, Object> environment) {
+        this.myName = Objects.requireNonNull(name, "name");
         this.myEnv = getMyEnv(environment);
-        this.myName = new SimpleJndiName("");
         this.serialContext = null;
     }
 
 
-    /**
-     * Create a context with the specified name+environment.
-     * Called only from GlassfishNamingManager.
-     */
-    public JavaURLContext(SimpleJndiName name, Hashtable<Object, Object> environment) {
+    public JavaURLContext(Hashtable<Object, Object> environment, SerialContext serialContext) {
+        this.myName = new SimpleJndiName("");
         this.myEnv = getMyEnv(environment);
-        this.myName = Objects.requireNonNull(name, "name");
-        this.serialContext = null;
+        this.serialContext = serialContext;
     }
 
 
     /**
-     * this constructor is called from SerialContext class
+     * Create a context with the same name and env.
      */
-    public JavaURLContext(Hashtable<Object, Object> environment, SerialContext serialContext) {
-        this.myEnv = getMyEnv(environment);
-        this.myName = new SimpleJndiName("");
-        this.serialContext = serialContext;
+    public JavaURLContext(JavaURLContext ctx) {
+        this.myName = ctx.myName;
+        this.myEnv = ctx.myEnv;
+        this.serialContext = null;
     }
 
 
-    public JavaURLContext(JavaURLContext ctx, SerialContext sctx) {
+    public JavaURLContext(JavaURLContext ctx, SerialContext serialContext) {
         this.myName = ctx.myName;
         this.myEnv = ctx.myEnv;
-        this.serialContext = sctx;
+        this.serialContext = serialContext;
     }
 
 
@@ -471,7 +468,7 @@ public Object removeFromEnvironment(String propName) throws NamingException {
      */
     @Override
     public void close() throws NamingException {
-        myEnv.clear();
+        this.myEnv.clear();
     }
 
     /**

appserver/common/glassfish-naming/src/main/java/com/sun/enterprise/naming/impl/ProviderManager.java

@@ -1,4 +1,5 @@
 /*
+ * Copyright (c) 2022 Contributors to the Eclipse Foundation
  * Copyright (c) 2006, 2018 Oracle and/or its affiliates. All rights reserved.
  *
  * This program and the accompanying materials are made available under the
@@ -34,44 +35,49 @@
  *
  * @author Sheetal Vartak
  */
-
-public class ProviderManager {
+public final class ProviderManager {
 
     private static ProviderManager providerManager;
 
-    private TransientContext rootContext = new TransientContext();
+    private final TransientContext rootContext = new TransientContext();
 
     private SerialContextProvider localProvider;
 
-    // Set lazily once initRemoteProvider is called.  Only available
-    // in server.
+    // Set lazily once initRemoteProvider is called.
+    // Only available in server.
     private ORB orb;
 
-    private ProviderManager() {}
+    private ProviderManager() {
+    }
+
 
     public synchronized static ProviderManager getProviderManager() {
-        if (providerManager  == null ) {
+        if (providerManager == null) {
             providerManager = new ProviderManager();
         }
         return providerManager;
     }
 
+
     public TransientContext getTransientContext() {
         return rootContext;
     }
 
+
     public synchronized SerialContextProvider getLocalProvider() {
         if (localProvider == null) {
             localProvider = LocalSerialContextProviderImpl.initProvider(rootContext);
         }
         return localProvider;
     }
 
+
     public Remote initRemoteProvider(ORB orb) throws RemoteException {
         this.orb = orb;
         return RemoteSerialContextProviderImpl.initSerialContextProvider(orb, rootContext);
     }
 
+
     ORB getORB() {
         return orb;
     }

appserver/common/glassfish-naming/src/main/java/com/sun/enterprise/naming/impl/SerialContext.java

@@ -76,7 +76,7 @@ public class SerialContext implements Context {
     @LogMessageInfo(message = "Exception during name lookup : {0}",
     cause = "App Server may not be running at port intended, or possible Network Error.",
     action = "Check to see if the AppServer is up and running on the port intended. The problem could be because of incorrect port. Check to see if you can access the host on which the AppServer running.")
-    public static final String EXCEPTION_DURING_LOOKUP = "AS-NAMING-00002";
+    private static final String EXCEPTION_DURING_LOOKUP = "AS-NAMING-00002";
 
     // Maximum number of recursive calls to lookup on comm error
     // Maximum number of recursive calls to lookup on comm error
@@ -304,7 +304,7 @@ private SerialContextProvider getProvider() throws NamingException {
     private ORB getORB() {
         if (orb == null) {
             ORBLocator orbHelper = services.getService(ORBLocator.class);
-            orb = orbHelper.getORB() ;
+            orb = orbHelper.getORB();
         }
         return orb;
     }
@@ -981,7 +981,8 @@ public Hashtable<Object, Object> getEnvironment() throws NamingException {
      */
     @Override
     public void close() throws NamingException {
-        myEnv = null;
+        this.myEnv.clear();
+        this.javaUrlContext.close();
     }
 
 

appserver/common/glassfish-naming/src/main/java/com/sun/enterprise/naming/impl/SerialContextProviderImpl.java

@@ -38,8 +38,7 @@ public Object lookup(String name) throws NamingException, RemoteException {
         } catch (NamingException ne) {
             throw ne;
         } catch (Exception e) {
-            RemoteException re = new RemoteException("", e);
-            throw re;
+            throw new RemoteException("Lookup failed for " + name, e);
         }
     }
 

appserver/common/glassfish-naming/src/main/java/com/sun/enterprise/naming/impl/TransientContext.java

@@ -55,6 +55,8 @@
  */
 public class TransientContext implements Context, Serializable {
 
+    private static final long serialVersionUID = -674500209229911786L;
+
     private static NameParser myParser = new SerialNameParser();
 
     private Hashtable<Object, Object> myEnv;
@@ -65,9 +67,6 @@ public class TransientContext implements Context, Serializable {
     // and only imposes a global lock on relatively infrequent updates.
     private static final ReadWriteLock lock = new ReentrantReadWriteLock();
 
-    public TransientContext() {
-    }
-
 
     /**
      * Create a subcontext with the specified name.
@@ -694,11 +693,13 @@ public Hashtable<Object, Object> getEnvironment() throws NamingException {
     }
 
     /**
-     * Invalidate the current environment.
+     * Resets the environemnt to null and clears all bindings.
+     * However the {@link TransientContext} instance can be used again.
      */
     @Override
     public void close() throws NamingException {
         myEnv = null;
+        bindings.clear();
     }
 
     /**
@@ -720,7 +721,7 @@ private static void print(Map<String,Object> ht) {
         }
     }
 
-    // Class for enumerating name/class pairs
+    /** Class for enumerating name/class pairs */
     static class RepNames implements NamingEnumeration<NameClassPair> {
 
         private final Map<String, String> nameToClassName = new HashMap<>();
@@ -769,7 +770,7 @@ public void close() {
         }
     }
 
-    // Class for enumerating bmesindings
+    /** Class for enumerating bmesindings */
     static class RepBindings implements NamingEnumeration<Binding> {
         Enumeration<String> names;
         Hashtable<String, Object> bindings;

appserver/common/glassfish-naming/src/test/java/com/sun/enterprise/naming/impl/GlassfishNamingManagerImplTest.java

@@ -23,8 +23,6 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
-import java.util.Properties;
-
 import javax.naming.Context;
 import javax.naming.InitialContext;
 import javax.naming.NamingException;
@@ -36,6 +34,7 @@
 import org.glassfish.api.invocation.InvocationManagerImpl;
 import org.glassfish.api.naming.JNDIBinding;
 import org.glassfish.api.naming.SimpleJndiName;
+import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -64,8 +63,16 @@ public class GlassfishNamingManagerImplTest {
 
     @BeforeAll
     public static void testCreateNewInitialContext() throws Exception {
-        Properties props = new Properties();
-        ctx = new InitialContext(props);
+        ctx = new InitialContext();
+    }
+
+
+    @AfterAll
+    public static void closeCtx() throws Exception {
+        if (ctx != null) {
+            ctx.close();
+        }
+        ProviderManager.getProviderManager().getTransientContext().close();
     }
 
 

appserver/common/glassfish-naming/src/test/java/com/sun/enterprise/naming/impl/SerialContextTest.java

@@ -17,6 +17,7 @@
 package com.sun.enterprise.naming.impl;
 
 import com.sun.enterprise.naming.impl.test.ServerExtension;
+
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -27,6 +28,7 @@
 import javax.naming.NamingException;
 
 import org.glassfish.internal.api.Globals;
+import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -52,6 +54,15 @@ public static void init() throws Exception {
     }
 
 
+    @AfterAll
+    public static void closeCtx() throws Exception {
+        if (ctx != null) {
+            ctx.close();
+        }
+        ProviderManager.getProviderManager().getTransientContext().close();
+    }
+
+
     @Test
     public void lookupOfNonExisting() throws Exception {
         assertThrows(NamingException.class, () -> ctx.lookup("java:comp/env/to-be-ingored"));
@@ -81,5 +92,6 @@ public void jdbc() throws Exception {
             () -> assertSame(fakeDS, ctx.lookup(jndiName)),
             () -> assertThat(names, hasItems("jdbc:derby:"))
         );
+        ctx.unbind(jndiName);
     }
 }