[tor-commits] [collector/master] Solves #19771: keep scheduling even in case of errors.

[karsten at torproject.org]

commit ffc38c3480da8f353859237301ed2a624e96b9b9
Author: iwakeh <iwakeh at torproject.org>
Date:   Mon Aug 8 09:43:59 2016 +0200

    Solves #19771: keep scheduling even in case of errors.
---
 .../torproject/collector/cron/CollecTorMain.java   |  4 +--
 .../java/org/torproject/collector/cron/Broken.java | 37 ++++++++++++++++++++++
 .../torproject/collector/cron/SchedulerTest.java   | 23 ++++++++++++++
 3 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/src/main/java/org/torproject/collector/cron/CollecTorMain.java b/src/main/java/org/torproject/collector/cron/CollecTorMain.java
index e9274ba..83d1ec3 100644
--- a/src/main/java/org/torproject/collector/cron/CollecTorMain.java
+++ b/src/main/java/org/torproject/collector/cron/CollecTorMain.java
@@ -40,8 +40,8 @@ public abstract class CollecTorMain implements Runnable {
     log.info("Starting {} module of CollecTor.", module());
     try {
       startProcessing();
-    } catch (ConfigurationException | RuntimeException ce) {
-      log.error("The {} module failed: {}", module(), ce.getMessage(), ce);
+    } catch (Throwable th) { // Catching all to prevent #19771
+      log.error("The {} module failed: {}", module(), th.getMessage(), th);
     }
     log.info("Terminating {} module of CollecTor.", module());
   }
diff --git a/src/test/java/org/torproject/collector/cron/Broken.java b/src/test/java/org/torproject/collector/cron/Broken.java
new file mode 100644
index 0000000..0a3bc93
--- /dev/null
+++ b/src/test/java/org/torproject/collector/cron/Broken.java
@@ -0,0 +1,37 @@
+/* Copyright 2016 The Tor Project
+ * See LICENSE for licensing information */
+
+package org.torproject.collector.cron;
+
+import org.torproject.collector.conf.Configuration;
+import org.torproject.collector.conf.ConfigurationException;
+
+import java.util.concurrent.atomic.AtomicInteger;
+
+public class Broken extends CollecTorMain {
+
+  static AtomicInteger count = new AtomicInteger(0);
+
+  public Broken(Configuration c) {
+    super(c);
+  }
+
+  @Override
+  public void startProcessing() throws ConfigurationException {
+    count.getAndIncrement();
+    try {
+      Thread.sleep(10);
+    } catch (Exception e) { /* ignored */ }
+    if (count.get() % 2 == 0) {
+      throw new Error("Throwing an Error.");
+    } else {
+      throw new RuntimeException("Throwing an Exception.");
+    }
+  }
+
+  @Override
+  public String module() {
+    return "broken";
+  }
+}
+
diff --git a/src/test/java/org/torproject/collector/cron/SchedulerTest.java b/src/test/java/org/torproject/collector/cron/SchedulerTest.java
index 9ed1ecf..c9f0c03 100644
--- a/src/test/java/org/torproject/collector/cron/SchedulerTest.java
+++ b/src/test/java/org/torproject/collector/cron/SchedulerTest.java
@@ -90,5 +90,28 @@ public class SchedulerTest {
     Scheduler.getInstance().shutdownScheduler();
     assertEquals(5, Counter.count.get());
   }
+
+  @Ignore("This test takes 180 seconds, which is too long.")
+  @Test()
+  public void testScheduleBrokenClass() throws Exception {
+    Map<Key, Class<? extends CollecTorMain>> ctms = new HashMap<>();
+    Configuration conf = new Configuration();
+    conf.load(new ByteArrayInputStream(runConfigProperties.getBytes()));
+    ctms.put(Key.TorperfActivated, Broken.class);
+    ctms.put(Key.BridgedescsActivated, Broken.class);
+    ctms.put(Key.RelaydescsActivated, Broken.class);
+    ctms.put(Key.ExitlistsActivated, Broken.class);
+    ctms.put(Key.UpdateindexActivated, Broken.class);
+    Field schedulerField = Scheduler.class.getDeclaredField("scheduler");
+    schedulerField.setAccessible(true);
+    ScheduledThreadPoolExecutor stpe = (ScheduledThreadPoolExecutor)
+        schedulerField.get(Scheduler.getInstance());
+    Scheduler.getInstance().scheduleModuleRuns(ctms, conf);
+    long sysNow = System.currentTimeMillis();
+    while (System.currentTimeMillis() - sysNow < 180_000) {
+      try { Thread.sleep(10_000);} catch (Exception e) { /* ignored */ }
+    }
+    assertEquals(15, Broken.count.intValue());
+  }
 }