[FP] ServiceContext fixes:

- Keep service context independent to address user session null pointer exceptions (#5507) - Review and document use of ServiceContext (#5540)
geonetwork · Nov 22, 2021 · 2296032 · 2296032
1 parent 1a61cfd
commit 2296032
Show file tree

Hide file tree

Showing 133 changed files with 5,816 additions and 4,092 deletions.
diff --git a/core/src/main/java/jeeves/interfaces/ApplicationHandler.java b/core/src/main/java/jeeves/interfaces/ApplicationHandler.java
@@ -29,12 +29,19 @@
 
 //=============================================================================
 
+/**
+ * Used to maintain registry of handlers for {@link jeeves.server.dispatchers.ServiceManager}.
+ */
 public interface ApplicationHandler {
-    public String getContextName();
+    /** Context name for registry lookup */
+    String getContextName();
 
-    public Object start(Element config, ServiceContext s) throws Exception;
+    /** Start application handler, returning application context managed in registry */
 
-    public void stop();
+    Object start(Element config, ServiceContext s) throws Exception;
+
+    /** Stop handler */
+    void stop();
 }
 
 //=============================================================================

diff --git a/core/src/main/java/jeeves/monitor/MonitorManager.java b/core/src/main/java/jeeves/monitor/MonitorManager.java
@@ -30,6 +30,7 @@
 import jeeves.constants.ConfigFile;
 import jeeves.server.context.ServiceContext;
 
+import jeeves.server.dispatchers.ServiceManager;
 import org.apache.commons.lang.StringUtils;
 import org.apache.log4j.LogManager;
 import org.fao.geonet.Util;
@@ -75,6 +76,13 @@ public class MonitorManager {
     private MetricsRegistry metricsRegistry;
     private JmxReporter jmxReporter;
 
+    /**
+     * Internal service context created during init, and cleaned up during shutdown.
+     *
+     * A distinct service context is required as health checks are performed in the background.
+     */
+    private ServiceContext monitorContext;
+
     public void init(ServletContext context, String baseUrl) {
 
         String webappName = "";
@@ -130,44 +138,64 @@ private HealthCheckRegistry lookUpHealthCheckRegistry(ServletContext context, St
         return tmpHealthCheckRegistry;
     }
 
-    public void initMonitorsForApp(ServiceContext context) {
-        createHealthCheck(context, criticalServiceContextHealthChecks, criticalHealthCheckRegistry, "critical health check");
-        createHealthCheck(context, warningServiceContextHealthChecks, warningHealthCheckRegistry, "warning health check");
-        createHealthCheck(context, expensiveServiceContextHealthChecks, expensiveHealthCheckRegistry, "expensive health check");
+    public void initMonitorsForApp(ServiceContext initContext) {
+        ServiceManager serviceManager = initContext.getBean(ServiceManager.class);
+        monitorContext = serviceManager.createServiceContext("monitor", initContext);
+
+        createHealthCheck(monitorContext, criticalServiceContextHealthChecks, criticalHealthCheckRegistry, "critical health check");
+        createHealthCheck(monitorContext, warningServiceContextHealthChecks, warningHealthCheckRegistry, "warning health check");
+        createHealthCheck(monitorContext, expensiveServiceContextHealthChecks, expensiveHealthCheckRegistry, "expensive health check");
 
         for (Class<MetricsFactory<Gauge<?>>> factoryClass : serviceContextGauges.keySet()) {
             Log.info(Log.ENGINE, "Instantiating : " + factoryClass.getName());
-            Gauge<?> instance = create(factoryClass, context, SERVICE_CONTEXT_GAUGE);
+            Gauge<?> instance = create(factoryClass, monitorContext, SERVICE_CONTEXT_GAUGE);
             serviceContextGauges.put(factoryClass, instance);
         }
         for (Class<MetricsFactory<Timer>> factoryClass : serviceContextTimers.keySet()) {
             Log.info(Log.ENGINE, "Instantiating : " + factoryClass.getName());
-            Timer instance = create(factoryClass, context, SERVICE_CONTEXT_TIMER);
+            Timer instance = create(factoryClass, monitorContext, SERVICE_CONTEXT_TIMER);
             serviceContextTimers.put(factoryClass, instance);
         }
         for (Class<MetricsFactory<Counter>> factoryClass : serviceContextCounters.keySet()) {
             Log.info(Log.ENGINE, "Instantiating : " + factoryClass.getName());
-            Counter instance = create(factoryClass, context, SERVICE_CONTEXT_COUNTER);
+            Counter instance = create(factoryClass, monitorContext, SERVICE_CONTEXT_COUNTER);
             serviceContextCounters.put(factoryClass, instance);
         }
         for (Class<MetricsFactory<Histogram>> factoryClass : serviceContextHistogram.keySet()) {
             Log.info(Log.ENGINE, "Instantiating : " + factoryClass.getName());
-            Histogram instance = create(factoryClass, context, SERVICE_CONTEXT_HISTOGRAM);
+            Histogram instance = create(factoryClass, monitorContext, SERVICE_CONTEXT_HISTOGRAM);
             serviceContextHistogram.put(factoryClass, instance);
         }
         for (Class<MetricsFactory<Meter>> factoryClass : serviceContextMeter.keySet()) {
             Log.info(Log.ENGINE, "Instantiating : " + factoryClass.getName());
-            Meter instance = create(factoryClass, context, SERVICE_CONTEXT_METER);
+            Meter instance = create(factoryClass, monitorContext, SERVICE_CONTEXT_METER);
             serviceContextMeter.put(factoryClass, instance);
         }
     }
 
+    /**
+     * Create and register health checks
+     *
+     * @param context
+     * @param checks factories used to create health checks
+     * @param registry registry listing heath checks
+     * @param type
+     */
     private void createHealthCheck(ServiceContext context, List<HealthCheckFactory> checks, HealthCheckRegistry registry, String type) {
+        ServiceManager serviceManager = context.getBean(ServiceManager.class);
+
         for (HealthCheckFactory healthCheck : checks) {
-            Log.info(Log.ENGINE, "Registering " + type + ": " + healthCheck.getClass().getName());
-            HealthCheck check = healthCheck.create(context);
-            healthCheckRegistry.register(check);
-            registry.register(check);
+            String factoryName = healthCheck.getClass().getName();
+            try {
+                HealthCheck check = healthCheck.create(context);
+
+                Log.info(Log.ENGINE, "Registering " + type + ": " + factoryName);
+                healthCheckRegistry.register(check);
+                registry.register(check);
+            }
+            catch (Throwable t){
+                Log.info(Log.ENGINE, "Unable to register " + type + ": " + factoryName);
+            }
         }
     }
 
@@ -306,6 +334,9 @@ public ResourceTracker getResourceTracker() {
     @PreDestroy
     public void shutdown() {
         Log.info(Log.ENGINE, "MonitorManager#shutdown");
+        if (monitorContext != null){
+            monitorContext.clear();
+        }
         if (resourceTracker != null) {
             resourceTracker.clean();
         }

diff --git a/core/src/main/java/jeeves/server/JeevesEngine.java b/core/src/main/java/jeeves/server/JeevesEngine.java
@@ -92,6 +92,8 @@ public class JeevesEngine {
     private Path _appPath;
     private int _maxUploadSize;
 
+    /** AppHandler service context used during init, tasks and background activities */
+    private ServiceContext appHandlerContext;
 
     public static void handleStartupError(Throwable e) {
         Log.fatal(Log.ENGINE, "Raised exception during init");
@@ -391,6 +393,13 @@ private void initDefault(Element defaults, ServiceManager serviceMan) throws Exc
     //---
     //---------------------------------------------------------------------------
 
+    /**
+     * Setup application hanlder using the provided handler definition.
+     *
+     * @param handler handler definition
+     * @param servlet jeeves servlet responsible for http distpatch
+     * @throws Exception
+     */
     private void initAppHandler(Element handler, JeevesServlet servlet) throws Exception {
         if (handler == null) {
             info("Handler not found");
@@ -413,20 +422,20 @@ private void initAppHandler(Element handler, JeevesServlet servlet) throws Excep
 
             ApplicationHandler h = (ApplicationHandler) c.newInstance();
 
-            ServiceContext srvContext = serviceMan.createServiceContext("AppHandler", appContext);
-            srvContext.setLanguage(_defaultLang);
-            srvContext.setLogger(_appHandLogger);
-            srvContext.setServlet(servlet);
-            srvContext.setAsThreadLocal();
+            appHandlerContext = serviceMan.createAppHandlerServiceContext(appContext);
+            appHandlerContext.setLanguage(_defaultLang);
+            appHandlerContext.setLogger(_appHandLogger);
+            appHandlerContext.setServlet(servlet);
+            appHandlerContext.setAsThreadLocal();
 
             try {
                 info("--- Starting handler --------------------------------------");
 
-                Object context = h.start(handler, srvContext);
+                Object context = h.start(handler, appHandlerContext);
 
                 _appHandlers.add(h);
                 serviceMan.registerContext(h.getContextName(), context);
-                monitorManager.initMonitorsForApp(srvContext);
+                monitorManager.initMonitorsForApp(appHandlerContext);
 
                 info("--- Handler started ---------------------------------------");
             } catch (Exception e) {
@@ -450,6 +459,9 @@ private void initAppHandler(Element handler, JeevesServlet servlet) throws Excep
                     serviceMan.setStartupErrors(errors);
                 }
             }
+            finally {
+                appHandlerContext.clearAsThreadLocal();
+            }
         }
     }
 
@@ -508,7 +520,11 @@ public void destroy() {
             info("Stopping handlers...");
             stopHandlers();
 
+            info("Clearing application handler context...");
+            appHandlerContext.clear();
+
             info("=== System stopped ========================================");
+
         } catch (Exception e) {
             error("Raised exception during destroy");
             error("  Exception : " + e);
@@ -525,7 +541,12 @@ public void destroy() {
 
     private void stopHandlers() throws Exception {
         for (ApplicationHandler h : _appHandlers) {
-            h.stop();
+            try {
+                h.stop();
+            } catch (Throwable unexpected){
+                _appHandLogger.error("Difficulty while stopping "+h.getContextName());
+                _appHandLogger.error(unexpected);
+            }
         }
     }
 

diff --git a/core/src/main/java/jeeves/server/context/BasicContext.java b/core/src/main/java/jeeves/server/context/BasicContext.java
@@ -43,9 +43,8 @@
 //=============================================================================
 
 /**
- * Contains a minimun context for a job execution (schedule, service etc...)
+ * Contains a minimum context for a job execution (schedule, service etc...)
  */
-
 public class BasicContext implements Logger {
 
     private final ConfigurableApplicationContext jeevesApplicationContext;