Refactored DatabaseCleanerImpl to use Timer and TimerTask.

components/net/sf/briar/db/DatabaseCleanerImpl.java

@@ -1,56 +1,45 @@
 package net.sf.briar.db;
 
+import java.util.Timer;
+import java.util.TimerTask;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
 import net.sf.briar.api.db.DbException;
 
-class DatabaseCleanerImpl implements DatabaseCleaner, Runnable {
+class DatabaseCleanerImpl extends TimerTask implements DatabaseCleaner {
 
 	private static final Logger LOG =
-		Logger.getLogger(DatabaseCleanerImpl.class.getName());
+			Logger.getLogger(DatabaseCleanerImpl.class.getName());
 
-	private Callback callback = null;
+	private volatile Callback callback = null;
-	private long msBetweenSweeps = 0L;
+	private volatile Timer timer = null;
-	private boolean stopped = false;
 
-	public synchronized void startCleaning(Callback callback,
+	public void startCleaning(Callback callback, long msBetweenSweeps) {
-			long msBetweenSweeps) {
 		this.callback = callback;
-		this.msBetweenSweeps = msBetweenSweeps;
+		timer = new Timer();
-		new Thread(this).start();
+		timer.scheduleAtFixedRate(this, 0L, msBetweenSweeps);
 	}
 
-	public synchronized void stopCleaning() {
+	public void stopCleaning() {
-		stopped = true;
+		if(timer == null) throw new IllegalStateException();
-		notifyAll();
+		timer.cancel();
 	}
 
 	public void run() {
-		while(true) {
+		if(callback == null) throw new IllegalStateException();
-			synchronized(this) {
+		try {
-				if(stopped) return;
+			if(callback.shouldCheckFreeSpace()) {
-				try {
+				callback.checkFreeSpaceAndClean();
-					if(callback.shouldCheckFreeSpace()) {
-						callback.checkFreeSpaceAndClean();
-					} else {
-						try {
-							wait(msBetweenSweeps);
-						} catch(InterruptedException e) {
-							if(LOG.isLoggable(Level.INFO))
-								LOG.info("Interrupted while waiting to clean");
-							Thread.currentThread().interrupt();
-							return;
-						}
-					}
-				} catch(DbException e) {
-					if(LOG.isLoggable(Level.WARNING))
-						LOG.warning(e.toString());
-				} catch(RuntimeException e) {
-					if(LOG.isLoggable(Level.WARNING))
-						LOG.warning(e.toString());
-				}
 			}
+		} catch(DbException e) {
+			if(LOG.isLoggable(Level.WARNING))
+				LOG.warning(e.toString());
+			throw new Error(e); // Kill the application
+		} catch(RuntimeException e) {
+			if(LOG.isLoggable(Level.WARNING))
+				LOG.warning(e.toString());
+			throw new Error(e); // Kill the application
 		}
 	}
 }

test/net/sf/briar/db/DatabaseCleanerImplTest.java

@@ -11,29 +11,51 @@ import org.junit.Test;
 
 public class DatabaseCleanerImplTest extends BriarTestCase {
 
+	@Test
+	public void testCleanerRunsPeriodically() throws Exception {
+		final CountDownLatch latch = new CountDownLatch(5);
+		Callback callback = new Callback() {
+
+			public void checkFreeSpaceAndClean() throws DbException {
+				latch.countDown();
+			}
+
+			public boolean shouldCheckFreeSpace() {
+				return true;
+			}
+		};
+		DatabaseCleanerImpl cleaner = new DatabaseCleanerImpl();
+		// Start the cleaner
+		cleaner.startCleaning(callback, 10L);
+		// The database should be cleaned five times (allow 5s for system load)
+		assertTrue(latch.await(5, TimeUnit.SECONDS));
+		// Stop the cleaner
+		cleaner.stopCleaning();
+	}
+
 	@Test
 	public void testStoppingCleanerWakesItUp() throws Exception {
 		final CountDownLatch latch = new CountDownLatch(1);
 		Callback callback = new Callback() {
 
 			public void checkFreeSpaceAndClean() throws DbException {
-				throw new IllegalStateException();
+				latch.countDown();
 			}
 
 			public boolean shouldCheckFreeSpace() {
-				latch.countDown();
+				return true;
-				return false;
 			}
 		};
 		DatabaseCleanerImpl cleaner = new DatabaseCleanerImpl();
 		long start = System.currentTimeMillis();
-		// Start the cleaner and check that shouldCheckFreeSpace() is called
+		// Start the cleaner
-		cleaner.startCleaning(callback, 30L * 1000L);
+		cleaner.startCleaning(callback, 10L * 1000L);
+		// The database should be cleaned once at startup
 		assertTrue(latch.await(5, TimeUnit.SECONDS));
 		// Stop the cleaner (it should be waiting between sweeps)
 		cleaner.stopCleaning();
 		long end = System.currentTimeMillis();
-		// Check that much less than 30 seconds expired
+		// Check that much less than 10 seconds expired
 		assertTrue(end - start < 10L * 1000L);
 	}
 }