From 034fb3dd93dc51b655f3d4e7eb5e14c048c0b086 Mon Sep 17 00:00:00 2001 From: Matthew Buckett Date: Tue, 14 Jan 2014 10:15:22 +0000 Subject: [PATCH] Don't get stuck in the loop. If we get an exception from the iterator don't catch it and re-try, this makes our loop safer from getting stuck and never returning. --- .../feed/module/cc/types/License.java | 2 +- .../feed/module/cc/types/LicenseTest.java | 31 +++++++++++++------ 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/rometools/feed/module/cc/types/License.java b/src/main/java/org/rometools/feed/module/cc/types/License.java index 83e1f80..06eac29 100644 --- a/src/main/java/org/rometools/feed/module/cc/types/License.java +++ b/src/main/java/org/rometools/feed/module/cc/types/License.java @@ -105,8 +105,8 @@ public class License { if (found == null && uri.startsWith("http://") && uri.toLowerCase().indexOf("creativecommons.org") != -1) { final Iterator it = License.lookupLicense.keySet().iterator(); while (it.hasNext() && found == null) { + final String key = (String) it.next(); try { - final String key = (String) it.next(); if (key.startsWith(CC_START)) { final String licensePath = key.substring(CC_START.length(), key.length()); final StringTokenizer tok = new StringTokenizer(licensePath, "/"); diff --git a/src/test/java/org/rometools/feed/module/cc/types/LicenseTest.java b/src/test/java/org/rometools/feed/module/cc/types/LicenseTest.java index f5841dc..46ee8f0 100644 --- a/src/test/java/org/rometools/feed/module/cc/types/LicenseTest.java +++ b/src/test/java/org/rometools/feed/module/cc/types/LicenseTest.java @@ -24,18 +24,24 @@ public class LicenseTest { public void testConcurrent() throws InterruptedException { final AtomicBoolean run = new AtomicBoolean(true); final AtomicLong type = new AtomicLong(0); + // Tracking any problems. final AtomicBoolean hadProblem = new AtomicBoolean(false); + final AtomicBoolean hadException = new AtomicBoolean(false); // This thread keeps on adding new licenses (not very realistic but shows the bug) Thread addNew = new Thread(){ @Override public void run() { - while(run.get()) { - License license = License.findByValue("http://creativecommons.org/licenses/"+ - type.incrementAndGet()+ "/1"); - if (license == null) { - hadProblem.set(true); + try { + while(run.get()) { + License license = License.findByValue("http://creativecommons.org/licenses/"+ + type.incrementAndGet()+ "/1"); + if (license == null) { + hadProblem.set(true); + } } + } catch (Exception e) { + hadException.set(true); } } }; @@ -45,12 +51,16 @@ public class LicenseTest { @Override public void run() { Random rnd = new Random(); - while(run.get()) { - License license = License.findByValue("http://creativecommons.org/licenses/"+ - rnd.nextInt(type.intValue())+"/1"); - if (license == null) { - hadProblem.set(true); + try { + while(run.get()) { + License license = License.findByValue("http://creativecommons.org/licenses/"+ + rnd.nextInt(type.intValue())+"/1"); + if (license == null) { + hadProblem.set(true); + } } + } catch (Exception e) { + hadException.set(true); } } }; @@ -66,6 +76,7 @@ public class LicenseTest { getExisting.join(50); // Check we didn't have any problems and they have both stopped. assertFalse(hadProblem.get()); + assertFalse(hadException.get()); assertFalse(addNew.isAlive()); assertFalse(getExisting.isAlive()); }