This is an automated email from the ASF dual-hosted git repository.

rombert pushed a commit to branch feature/SLING-7811
in repository 
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-base.git

commit 44def0e802e5eebeaef36a0827d0b5d9fde999b5
Author: Robert Munteanu <[email protected]>
AuthorDate: Tue Aug 4 15:03:54 2020 +0200

    SLING-7811 - NPE when repository is starting up due to repository manager 
shutdown
    
    - simplify flow control by removing 'stopRequested' and 'throwable'
    - no longer use interrupts to stop the repository
    - wait for the repository to start fully before stopping, with a deadline 
of 5 minutes
---
 .../jcr/base/AbstractSlingRepositoryManager.java   | 62 ++++++++++++----------
 1 file changed, 35 insertions(+), 27 deletions(-)

diff --git 
a/src/main/java/org/apache/sling/jcr/base/AbstractSlingRepositoryManager.java 
b/src/main/java/org/apache/sling/jcr/base/AbstractSlingRepositoryManager.java
index dbda40d..dc7c894 100644
--- 
a/src/main/java/org/apache/sling/jcr/base/AbstractSlingRepositoryManager.java
+++ 
b/src/main/java/org/apache/sling/jcr/base/AbstractSlingRepositoryManager.java
@@ -120,8 +120,6 @@ public abstract class AbstractSlingRepositoryManager {
 
     private volatile Thread startupThread;
 
-    private volatile boolean stopRequested;
-
     volatile ServiceTracker<RepositoryMount, RepositoryMount> mountTracker;
 
     /**
@@ -509,7 +507,6 @@ public abstract class AbstractSlingRepositoryManager {
     }
 
     private void initializeAndRegisterRepositoryService() {
-        Throwable t = null;
         try {
             log.debug("start: calling acquireRepository()");
             Repository newRepo = this.acquireRepository();
@@ -518,11 +515,6 @@ public abstract class AbstractSlingRepositoryManager {
                 // ensure we really have the repository
                 log.debug("start: got a Repository");
                 this.repository = newRepo;
-                if ( stopRequested ) {
-                    log.debug("Stop requested, cancelling initialisation and 
stopping repository");
-                    stop();
-                    return;
-                }
                 synchronized ( this.repoInitLock ) {
                     this.masterSlingRepository = 
this.create(this.bundleContext.getBundle());
 
@@ -533,25 +525,21 @@ public abstract class AbstractSlingRepositoryManager {
                     try {
                         
executeRepositoryInitializers(this.masterSlingRepository);
                     } catch(Throwable e) {
-                        t = e;
-                        log.error("Exception in a SlingRepositoryInitializer, 
SlingRepository service registration aborted", t);
+                        log.error("Exception in a SlingRepositoryInitializer, 
SlingRepository service registration aborted", e);
+                        stop();
+                        return;
                     }
 
                     log.debug("start: calling registerService()");
                     this.repositoryService = registerService();
 
-                    log.debug("start: registerService() successful, 
registration=" + repositoryService);
+                    log.debug("start: registerService() successful, 
registration={}", repositoryService);
                 }
             }
         } catch (Throwable e) {
             // consider an uncaught problem an error
             log.error("start: Uncaught Throwable trying to access Repository, 
calling stopRepository()", e);
-            t = e;
-        } finally {
-            if (t != null) {
-                // repository might be partially started, stop anything left
-                stop();
-            }
+            stop();
         }
     }
 
@@ -595,16 +583,9 @@ public abstract class AbstractSlingRepositoryManager {
      * This method must be called if overwritten by implementations !!
      */
     protected final void stop() {
-
-        stopRequested = true;
-        if ( startupThread != null && startupThread != Thread.currentThread() 
) {
-            try {
-                startupThread.interrupt();
-                startupThread.join();
-            } catch (InterruptedException e) {
-                log.debug("Interrupted while waiting for the " + 
startupThread.getName() + " to complete.", e);
-                Thread.currentThread().interrupt();
-            }
+        log.info("Stop requested");
+        if ( startupThread != Thread.currentThread() ) {
+            waitForStartupThreadToComplete();
             startupThread = null;
         }
 
@@ -670,6 +651,33 @@ public abstract class AbstractSlingRepositoryManager {
         this.bundleContext = null;
     }
 
+    private void waitForStartupThreadToComplete() {
+        // TODO - instance variables, maybe configurable
+        int maxWaitCount = 5;
+        long waitMillis = TimeUnit.MINUTES.toMillis(1);
+
+        try {
+            // Oak does play well with interrupted exceptions, so avoid that 
at all costs
+            // https://jackrabbit.apache.org/oak/docs/dos_and_donts.html
+            for ( int i = 0; i < maxWaitCount; i++ ) {
+                log.info("Waiting {} millis for {} to complete, attempt 
{}/{}.", waitMillis, startupThread.getName(), (i + 1), maxWaitCount);
+                startupThread.join(waitMillis);
+                if ( !startupThread.isAlive() ) {
+                    log.info("{} not alive, proceeding", 
startupThread.getName());
+                    break;
+                }
+            }
+        } catch (InterruptedException e) {
+            log.debug("Interrupted while waiting for the " + 
startupThread.getName() + " to complete.", e);
+            Thread.currentThread().interrupt();
+        }
+        
+        if ( startupThread.isAlive() ) {
+            log.warn("Proceeding even though {} is still running, behaviour is 
undefined.", startupThread.getName());
+            // TODO - log stack trace
+        }
+    }
+
     private static final class SlingRepositoryInitializerInfo implements 
Comparable<SlingRepositoryInitializerInfo> {
 
         final SlingRepositoryInitializer initializer;

Reply via email to