You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by bd...@apache.org on 2009/09/11 11:43:51 UTC

svn commit: r813746 - in /sling/trunk/installer/jcr/jcrinstall/src: main/java/org/apache/sling/jcr/jcrinstall/impl/ test/java/org/apache/sling/jcr/jcrinstall/impl/

Author: bdelacretaz
Date: Fri Sep 11 09:43:50 2009
New Revision: 813746

URL: http://svn.apache.org/viewvc?rev=813746&view=rev
Log:
SLING-1078 - improved handling of JcrInstaller background thread, and fix tests that failed due to changes in go idle behavior

Modified:
    sling/trunk/installer/jcr/jcrinstall/src/main/java/org/apache/sling/jcr/jcrinstall/impl/JcrInstaller.java
    sling/trunk/installer/jcr/jcrinstall/src/test/java/org/apache/sling/jcr/jcrinstall/impl/MiscUtil.java
    sling/trunk/installer/jcr/jcrinstall/src/test/java/org/apache/sling/jcr/jcrinstall/impl/ResourceDetectionTest.java

Modified: sling/trunk/installer/jcr/jcrinstall/src/main/java/org/apache/sling/jcr/jcrinstall/impl/JcrInstaller.java
URL: http://svn.apache.org/viewvc/sling/trunk/installer/jcr/jcrinstall/src/main/java/org/apache/sling/jcr/jcrinstall/impl/JcrInstaller.java?rev=813746&r1=813745&r2=813746&view=diff
==============================================================================
--- sling/trunk/installer/jcr/jcrinstall/src/main/java/org/apache/sling/jcr/jcrinstall/impl/JcrInstaller.java (original)
+++ sling/trunk/installer/jcr/jcrinstall/src/main/java/org/apache/sling/jcr/jcrinstall/impl/JcrInstaller.java Fri Sep 11 09:43:50 2009
@@ -59,7 +59,7 @@
  *      name="service.vendor"
  *      value="The Apache Software Foundation"
  */
-public class JcrInstaller implements Runnable, EventListener {
+public class JcrInstaller implements EventListener {
 	public static final long RUN_LOOP_DELAY_MSEC = 500L;
 	public static final String URL_SCHEME = "jcrinstall";
 
@@ -123,9 +123,6 @@
     /** Session shared by all WatchedFolder */
     private Session session;
     
-    /** Used to stop background thread when deactivated */
-    private int deactivationCounter = 1;
-    
     /** The root folders that we watch */
     private String [] roots;
     
@@ -145,6 +142,29 @@
     
     /** Timer used to call updateFoldersList() */
     private final RescanTimer updateFoldersListTimer = new RescanTimer();
+    
+    /** Thread that can be cleanly stopped with a flag */
+    static int bgThreadCounter;
+    class StoppableThread extends Thread {
+        boolean active = true;
+        StoppableThread() {
+            synchronized (JcrInstaller.class) {
+                setName("JcrInstaller." + (++bgThreadCounter));
+            }
+            setDaemon(true);
+        }
+        
+        @Override
+        public final void run() {
+            log.info("Background thread {} starting", Thread.currentThread().getName());
+            while(active) {
+                runOneCycle();
+            }
+            log.info("Background thread {} done", Thread.currentThread().getName());
+            counters[RUN_LOOP_COUNTER] = -1;
+        }
+    };
+    private StoppableThread backgroundThread;
 
     protected void activate(ComponentContext context) throws Exception {
     	
@@ -229,16 +249,27 @@
     	log.info("Registering {} resources with OSGi installer: {}", resources.size(), resources);
     	installer.registerResources(resources, URL_SCHEME);
     	
-    	final Thread t = new Thread(this, getClass().getSimpleName() + "." + deactivationCounter);
-    	t.setDaemon(true);
-    	t.start();
+    	if(backgroundThread != null) {
+    	    throw new IllegalStateException("Expected backgroundThread to be null in activate()");
+    	}
+        backgroundThread = new StoppableThread();
+        backgroundThread.start();
     }
     
     protected void deactivate(ComponentContext context) {
     	log.info("deactivate()");
 
+    	final long timeout = 30000L;
+    	try {
+            backgroundThread.active = false;
+            log.debug("Waiting for " + backgroundThread.getName() + " Thread to end...");
+            backgroundThread.join(timeout);
+            backgroundThread = null;
+    	} catch(InterruptedException iex) {
+    	    throw new IllegalStateException("backgroundThread.join interrupted after " + timeout + " msec");
+    	}
+        
         try {
-            deactivationCounter++;
             folderNameFilter = null;
             watchedFolders = null;
             converters.clear();
@@ -415,60 +446,54 @@
     }
 
     /** Run periodic scans of our watched folders, and watch for folders creations/deletions */
-    public void run() {
-        log.info("Background thread {} starting", Thread.currentThread().getName());
-        final int savedCounter = deactivationCounter;
-        while(savedCounter == deactivationCounter) {
-            try {
-                // Rescan WatchedFolders if needed
-                final boolean scanWf = WatchedFolder.getRescanTimer().expired(); 
-                if(scanWf) {
-                    for(WatchedFolder wf : watchedFolders) {
-                        if(!wf.needsScan()) {
-                            continue;
-                        }
-                        WatchedFolder.getRescanTimer().reset();
-                        counters[SCAN_FOLDERS_COUNTER]++;
-                        final WatchedFolder.ScanResult sr = wf.scan();
-                        for(InstallableResource r : sr.toRemove) {
-                            log.info("Removing resource from OSGi installer: {}",r);
-                            installer.removeResource(r);
-                        }
-                        for(InstallableResource r : sr.toAdd) {
-                            log.info("Registering resource with OSGi installer: {}",r);
-                            installer.addResource(r);
-                        }
+    public void runOneCycle() {
+        try {
+            // Rescan WatchedFolders if needed
+            final boolean scanWf = WatchedFolder.getRescanTimer().expired(); 
+            if(scanWf) {
+                for(WatchedFolder wf : watchedFolders) {
+                    if(!wf.needsScan()) {
+                        continue;
                     }
-                }
-
-                // Update list of WatchedFolder if we got any relevant events,
-                // or if there were any WatchedFolder events
-                if(scanWf || updateFoldersListTimer.expired()) {
-                    updateFoldersListTimer.reset();
-                    counters[UPDATE_FOLDERS_LIST_COUNTER]++;
-                    final List<InstallableResource> toRemove = updateFoldersList();
-                    for(InstallableResource r : toRemove) {
-                        log.info("Removing resource from OSGi installer (folder deleted): {}",r);
+                    WatchedFolder.getRescanTimer().reset();
+                    counters[SCAN_FOLDERS_COUNTER]++;
+                    final WatchedFolder.ScanResult sr = wf.scan();
+                    for(InstallableResource r : sr.toRemove) {
+                        log.info("Removing resource from OSGi installer: {}",r);
                         installer.removeResource(r);
                     }
+                    for(InstallableResource r : sr.toAdd) {
+                        log.info("Registering resource with OSGi installer: {}",r);
+                        installer.addResource(r);
+                    }
                 }
+            }
 
-                try {
-                    Thread.sleep(RUN_LOOP_DELAY_MSEC);
-                } catch(InterruptedException ignore) {
-                }
-                
-            } catch(Exception e) {
-                log.warn("Exception in run()", e);
-                try {
-                    Thread.sleep(RUN_LOOP_DELAY_MSEC);
-                } catch(InterruptedException ignore) {
+            // Update list of WatchedFolder if we got any relevant events,
+            // or if there were any WatchedFolder events
+            if(scanWf || updateFoldersListTimer.expired()) {
+                updateFoldersListTimer.reset();
+                counters[UPDATE_FOLDERS_LIST_COUNTER]++;
+                final List<InstallableResource> toRemove = updateFoldersList();
+                for(InstallableResource r : toRemove) {
+                    log.info("Removing resource from OSGi installer (folder deleted): {}",r);
+                    installer.removeResource(r);
                 }
             }
-            counters[RUN_LOOP_COUNTER]++;
+
+            try {
+                Thread.sleep(RUN_LOOP_DELAY_MSEC);
+            } catch(InterruptedException ignore) {
+            }
+            
+        } catch(Exception e) {
+            log.warn("Exception in run()", e);
+            try {
+                Thread.sleep(RUN_LOOP_DELAY_MSEC);
+            } catch(InterruptedException ignore) {
+            }
         }
-        log.info("Background thread {} done", Thread.currentThread().getName());
-        counters[RUN_LOOP_COUNTER] = -1;
+        counters[RUN_LOOP_COUNTER]++;
     }
     
     long [] getCounters() {

Modified: sling/trunk/installer/jcr/jcrinstall/src/test/java/org/apache/sling/jcr/jcrinstall/impl/MiscUtil.java
URL: http://svn.apache.org/viewvc/sling/trunk/installer/jcr/jcrinstall/src/test/java/org/apache/sling/jcr/jcrinstall/impl/MiscUtil.java?rev=813746&r1=813745&r2=813746&view=diff
==============================================================================
--- sling/trunk/installer/jcr/jcrinstall/src/test/java/org/apache/sling/jcr/jcrinstall/impl/MiscUtil.java (original)
+++ sling/trunk/installer/jcr/jcrinstall/src/test/java/org/apache/sling/jcr/jcrinstall/impl/MiscUtil.java Fri Sep 11 09:43:50 2009
@@ -123,15 +123,16 @@
         return cc;
     }
     
-    static private void waitForCycles(JcrInstaller installer, int nCycles, long timeoutMsec) throws Exception {
-        final long endCycles = installer.getCounters()[JcrInstaller.RUN_LOOP_COUNTER];
+    static private void waitForCycles(JcrInstaller installer, long initialCycleCount, int expectedCycles, long timeoutMsec) throws Exception {
         final long endTime = System.currentTimeMillis() + timeoutMsec;
+        long cycles = 0;
         while(System.currentTimeMillis() < endTime) {
-            if(installer.getCounters()[JcrInstaller.RUN_LOOP_COUNTER] > endCycles) {
+            cycles = installer.getCounters()[JcrInstaller.RUN_LOOP_COUNTER] - initialCycleCount;
+            if(cycles >= expectedCycles) {
                 return;
             }
         }
-        throw new Exception("did not get " + nCycles + " installer cycles in " + timeoutMsec + " msec"); 
+        throw new Exception("did not get " + expectedCycles + " installer cycles in " + timeoutMsec + " msec, got " + cycles); 
     }
     
     /** Get the WatchedFolders of supplied JcrInstaller */
@@ -144,12 +145,14 @@
     
     /** Wait long enough for all changes in content to be processed by JcrInstaller */ 
     static void waitAfterContentChanges(EventHelper eventHelper, JcrInstaller installer) throws Exception {
+        final long startCycles = installer.getCounters()[JcrInstaller.RUN_LOOP_COUNTER];
+        
         // First wait for all JCR events to be delivered
         eventHelper.waitForEvents(5000L);
         // RescanTimer causes a SCAN_DELAY_MSEC wait after JCR events are received
         Thread.sleep(RescanTimer.SCAN_DELAY_MSEC * 2);
-        // And wait for 2 complete JcrInstaller run cycles, to be on the safe side
-        MiscUtil.waitForCycles(installer, 3, 5000L);
+        // And wait for one JcrInstaller run cycle
+        MiscUtil.waitForCycles(installer, startCycles, 1, 5000L);
     }
     
     /** Wait until installer thread exits */

Modified: sling/trunk/installer/jcr/jcrinstall/src/test/java/org/apache/sling/jcr/jcrinstall/impl/ResourceDetectionTest.java
URL: http://svn.apache.org/viewvc/sling/trunk/installer/jcr/jcrinstall/src/test/java/org/apache/sling/jcr/jcrinstall/impl/ResourceDetectionTest.java?rev=813746&r1=813745&r2=813746&view=diff
==============================================================================
--- sling/trunk/installer/jcr/jcrinstall/src/test/java/org/apache/sling/jcr/jcrinstall/impl/ResourceDetectionTest.java (original)
+++ sling/trunk/installer/jcr/jcrinstall/src/test/java/org/apache/sling/jcr/jcrinstall/impl/ResourceDetectionTest.java Fri Sep 11 09:43:50 2009
@@ -190,20 +190,25 @@
     	assertRegistered(path, true);
     	
     	// Setup a known value for the config
+        osgiInstaller.clearRecordedCalls();
     	final String key = "foo" + System.currentTimeMillis();
     	final String value = "value" + System.currentTimeMillis();
     	((Node)session.getItem(path)).setProperty(key, value);
     	session.save();
         MiscUtil.waitAfterContentChanges(eventHelper, installer);
+        assertEquals("Expected one OsgiInstaller call for initial config change",
+                1, osgiInstaller.getRecordedCalls().size());
     			
     	// Make a change that does not influence the configs's digest,
     	// and verify that no OSGi registrations result
-    	int nCalls = osgiInstaller.getRecordedCalls().size();
+        osgiInstaller.clearRecordedCalls();
     	((Node)session.getItem(path)).setProperty(key, value);
     	session.save();
         MiscUtil.waitAfterContentChanges(eventHelper, installer);
-        assertEquals("Expected no OsgiInstaller calls for no-impact config change",
-        		nCalls, osgiInstaller.getRecordedCalls().size());
+        assertEquals(
+                "Expected no OsgiInstaller calls for no-impact config change, got " 
+                + osgiInstaller.getRecordedCalls(),
+        		0, osgiInstaller.getRecordedCalls().size());
         
         // Make a content change -> resource must be re-registered
         osgiInstaller.clearRecordedCalls();