Merge pull request #4602 from jbee/APPSERV-114-race-invocation-manager

APPSERV-114 Addresses possible sources of Race Conditions in InvocationManager
payara · Apr 22, 2020 · fbbd633 · fbbd633
2 parents ca93e01 + b5bdaa2
commit fbbd633
Show file tree

Hide file tree

Showing 7 changed files with 766 additions and 252 deletions.
diff --git a/...s/microprofile/metrics/src/main/java/fish/payara/microprofile/metrics/MetricsService.java b/...s/microprofile/metrics/src/main/java/fish/payara/microprofile/metrics/MetricsService.java
@@ -60,6 +60,7 @@
 import org.glassfish.api.admin.ServerEnvironment;
 import org.glassfish.api.event.EventListener;
 import org.glassfish.api.event.Events;
+import org.glassfish.api.invocation.ComponentInvocation;
 import org.glassfish.api.invocation.InvocationManager;
 import org.glassfish.hk2.api.ServiceLocator;
 import org.glassfish.hk2.runlevel.RunLevel;
@@ -396,15 +397,16 @@ private void deregisterApplication(String applicationName) {
     public String getApplicationName() {
         InvocationManager invocationManager = Globals.getDefaultBaseServiceLocator()
                 .getService(InvocationManager.class);
-        if (invocationManager.getCurrentInvocation() == null) {
+        ComponentInvocation current = invocationManager.getCurrentInvocation();
+        if (current == null) {
             return invocationManager.peekAppEnvironment().getName();
         }
-        String appName = invocationManager.getCurrentInvocation().getAppName();
+        String appName = current.getAppName();
         if (appName == null) {
-            appName = invocationManager.getCurrentInvocation().getModuleName();
+            appName = current.getModuleName();
         }
         if (appName == null) {
-            appName = invocationManager.getCurrentInvocation().getComponentId();
+            appName = current.getComponentId();
         }
         return appName;
     }

diff --git a/appserver/web/weld-integration/src/main/java/org/glassfish/weld/WeldDeployer.java b/appserver/web/weld-integration/src/main/java/org/glassfish/weld/WeldDeployer.java
@@ -201,7 +201,7 @@ public class WeldDeployer extends SimpleDeployer<WeldContainer, WeldApplicationC
 
     @Inject
     private Deployment deployment;
-    
+
     @Inject
     private PayaraExecutorService executorService;
 
@@ -298,7 +298,7 @@ public WeldApplicationContainer load(WeldContainer container, DeploymentContext
 
             ProxyServices proxyServices = new ProxyServicesImpl(services);
             deploymentImpl.getServices().add(ProxyServices.class, proxyServices);
-            
+
             ExecutorServices executorServices = new ExecutorServicesImpl(executorService);
             deploymentImpl.getServices().add(ExecutorServices.class, executorServices);
 
@@ -318,7 +318,7 @@ public WeldApplicationContainer load(WeldContainer container, DeploymentContext
         externalConfiguration.setBeanIndexOptimization(!deployParams.isAvailabilityEnabled());
         externalConfiguration.setNonPortableMode(false);
         configureConcurrentDeployment(context, externalConfiguration);
-        
+
         deploymentImpl.getServices().add(ExternalConfiguration.class, externalConfiguration);
 
         BeanDeploymentArchive beanDeploymentArchive = deploymentImpl.getBeanDeploymentArchiveForArchive(archiveName);
@@ -499,7 +499,7 @@ private void processApplicationLoaded(ApplicationInfo applicationInfo) {
             // Get current TCL
             ClassLoader oldTCL = Thread.currentThread().getContextClassLoader();
 
-            invocationManager.pushAppEnvironment(() ->  applicationInfo.getName());
+            invocationManager.pushAppEnvironment(applicationInfo::getName);
 
             ComponentInvocation componentInvocation = createComponentInvocation(applicationInfo);
 
@@ -553,7 +553,7 @@ private void processApplicationStopped(ApplicationInfo applicationInfo) {
         try {
             WeldBootstrap bootstrap = applicationInfo.getTransientAppMetaData(WELD_BOOTSTRAP, WeldBootstrap.class);
             if (bootstrap != null) {
-                invocationManager.pushAppEnvironment(() ->  applicationInfo.getName());
+                invocationManager.pushAppEnvironment(applicationInfo::getName);
 
                 try {
                     doBootstrapShutdown(applicationInfo);
@@ -868,7 +868,7 @@ private void registerProbeExtension(ExternalConfigurationImpl externalConfigurat
         externalConfiguration.setProbeAllowRemoteAddress(PROBE_ALLOW_REMOTE_ADDRESS);
         deploymentImpl.addDynamicExtension(createProbeExtension());
     }
-    
+
     private void configureConcurrentDeployment(DeploymentContext context, ExternalConfigurationImpl configuration) {
         configuration.setConcurrentDeployment(WeldUtils.isConcurrentDeploymentEnabled());
         configuration.setPreLoaderThreadPoolSize(WeldUtils.getPreLoaderThreads());

diff --git a/nucleus/common/glassfish-api/pom.xml b/nucleus/common/glassfish-api/pom.xml
@@ -105,7 +105,12 @@
             <artifactId>junit</artifactId>
             <scope>test</scope>
             <optional>true</optional>
-      </dependency>        
+        </dependency>
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-core</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 </project>
 
diff --git a/.../common/glassfish-api/src/main/java/org/glassfish/api/invocation/ComponentInvocation.java b/.../common/glassfish-api/src/main/java/org/glassfish/api/invocation/ComponentInvocation.java
@@ -37,7 +37,7 @@
  * only if the new code is made subject to such option by the copyright
  * holder.
  */
-// Portions Copyright [2019] [Payara Foundation and/or its affiliates]
+// Portions Copyright [2019-2020] [Payara Foundation and/or its affiliates]
 
 package org.glassfish.api.invocation;
 
@@ -54,10 +54,10 @@
 public class ComponentInvocation implements Cloneable {
 
     public enum ComponentInvocationType {
-        SERVLET_INVOCATION, 
-        EJB_INVOCATION, 
-        APP_CLIENT_INVOCATION, 
-        UN_INITIALIZED, 
+        SERVLET_INVOCATION,
+        EJB_INVOCATION,
+        APP_CLIENT_INVOCATION,
+        UN_INITIALIZED,
         SERVICE_STARTUP
     }
 
@@ -140,7 +140,7 @@ public ComponentInvocation(String componentId, ComponentInvocationType invocatio
     public ComponentInvocationType getInvocationType() {
         return invocationType;
     }
-    
+
     public void setInvocationType(ComponentInvocationType invocationType) {
         this.invocationType = invocationType;
     }
@@ -160,15 +160,15 @@ public void setPreInvokeDone(boolean value) {
     public Boolean getAuth() {
         return auth;
     }
-    
+
     public void setAuth(Boolean auth) {
         this.auth = auth;
     }
 
     public void setAuth(boolean value) {
         auth = value;
     }
-    
+
     public boolean isPreInvokeDoneStatus() {
         return preInvokeDoneStatus;
     }
@@ -180,7 +180,7 @@ public void setPreInvokeDoneStatus(boolean preInvokeDoneStatus) {
     public Object getInstance() {
         return instance;
     }
-    
+
     public void setInstance(Object instance) {
         this.instance = instance;
     }
@@ -196,11 +196,11 @@ public void setInstanceName(String instanceName) {
     public String getComponentId() {
         return this.componentId;
     }
-    
+
     public void setComponentId(String componentId) {
         this.componentId = componentId;
     }
-    
+
     public Object getJndiEnvironment() {
         return jndiEnvironment;
     }
@@ -220,7 +220,7 @@ public Object getJNDIEnvironment() {
     public Object getContainer() {
         return container;
     }
-    
+
     public void setContainer(Object container) {
         this.container = container;
     }
@@ -236,11 +236,11 @@ public Object getTransaction() {
     public void setTransaction(Object t) {
         this.transaction = t;
     }
-    
+
     public void setTransactionCompleting(boolean transactionCompleting) {
         this.transactionCompleting = transactionCompleting;
     }
-    
+
     public Map<Class<?>, Object> getRegistry() {
         return registry;
     }
@@ -258,7 +258,7 @@ public void setRegistry(Map<Class<?>, Object> registry) {
     public String getAppName() {
         return appName;
     }
-    
+
     public void setAppName(String appName) {
         this.appName = appName;
     }
@@ -270,7 +270,7 @@ public void setAppName(String appName) {
     public String getModuleName() {
         return moduleName;
     }
-    
+
     public void setModuleName(String moduleName) {
         this.moduleName = moduleName;
     }
@@ -383,5 +383,16 @@ public ComponentInvocation clone() {
 
         return newInv;
     }
-
+
+    @Override
+    public String toString() {
+        StringBuilder str = new StringBuilder();
+        str.append(Integer.toHexString(System.identityHashCode(this))).append('@').append(getClass().getName()).append('\n');
+        str.append("\tcomponentId=").append(componentId).append('\n');
+        str.append("\ttype=").append(invocationType).append('\n');
+        str.append("\tinstance=").append(instanceName != null ? instanceName : String.valueOf(instance)).append('\n');
+        str.append("\tcontainer=").append(container).append('\n');
+        str.append("\tappName=").append(appName).append('\n');
+        return str.toString();
+    }
 }
diff --git a/.../glassfish-api/src/main/java/org/glassfish/api/invocation/ComponentInvocationHandler.java b/.../glassfish-api/src/main/java/org/glassfish/api/invocation/ComponentInvocationHandler.java
@@ -37,6 +37,7 @@
  * only if the new code is made subject to such option by the copyright
  * holder.
  */
+// Portions Copyright 2020 Payara Foundation and/or its affiliates
 package org.glassfish.api.invocation;
 
 import org.glassfish.api.invocation.ComponentInvocation.ComponentInvocationType;
@@ -53,43 +54,23 @@
 public interface ComponentInvocationHandler {
 
     /**
-     * Called <b>before</b> the curInv is pushed into the invocation stack.
-     *
-     * @param invType
-     * @param prevInv
-     * @param newInv
-     * @throws InvocationException
+     * Called <b>before</b> the cur is pushed into the invocation stack.
      */
-    void beforePreInvoke(ComponentInvocationType invType, ComponentInvocation prevInv, ComponentInvocation newInv) throws InvocationException;
+    void beforePreInvoke(ComponentInvocationType type, ComponentInvocation prev, ComponentInvocation cur) throws InvocationException;
 
     /**
-     * Called <b>after</b> the curInv has been pushed into the invocation stack.
-     *
-     * @param invType
-     * @param prevInv
-     * @param curInv
-     * @throws InvocationException
+     * Called <b>after</b> the cur has been pushed into the invocation stack.
      */
-    void afterPreInvoke(ComponentInvocationType invType, ComponentInvocation prevInv, ComponentInvocation curInv) throws InvocationException;
+    void afterPreInvoke(ComponentInvocationType type, ComponentInvocation prev, ComponentInvocation cur) throws InvocationException;
 
     /**
-     * Called <b>before</b> the curInv has been popped from the invocation stack.
-     *
-     * @param invType
-     * @param prevInv
-     * @param curInv
-     * @throws InvocationException
+     * Called <b>before</b> the cur has been popped from the invocation stack.
      */
-    void beforePostInvoke(ComponentInvocationType invType, ComponentInvocation prevInv, ComponentInvocation curInv) throws InvocationException;
+    void beforePostInvoke(ComponentInvocationType type, ComponentInvocation prev, ComponentInvocation cur) throws InvocationException;
 
     /**
      * Called <b>after</b> the curInv has been popped from the invocation stack.
-     *
-     * @param invType
-     * @param prevInv
-     * @param curInv
-     * @throws InvocationException
      */
-    void afterPostInvoke(ComponentInvocationType invType, ComponentInvocation prevInv, ComponentInvocation curInv) throws InvocationException;
+    void afterPostInvoke(ComponentInvocationType type, ComponentInvocation prev, ComponentInvocation cur) throws InvocationException;
 
 }