From e03213b1c6e0f76217ec7a93c64767f0fdcb4d47 Mon Sep 17 00:00:00 2001 From: rbarker Date: Tue, 1 Jul 2014 13:49:27 -0700 Subject: [PATCH 1/2] Fix for ConcurrentModificationException --- .../jersey/client/ahc/AhcClientHandler.java | 47 +++++++++------ .../client/ahc/tests/tests/CookieTest.java | 58 +++++++++++++++++++ 2 files changed, 86 insertions(+), 19 deletions(-) diff --git a/src/main/java/org/sonatype/spice/jersey/client/ahc/AhcClientHandler.java b/src/main/java/org/sonatype/spice/jersey/client/ahc/AhcClientHandler.java index 39f8d5a..85b0a96 100644 --- a/src/main/java/org/sonatype/spice/jersey/client/ahc/AhcClientHandler.java +++ b/src/main/java/org/sonatype/spice/jersey/client/ahc/AhcClientHandler.java @@ -12,9 +12,11 @@ package org.sonatype.spice.jersey.client.ahc; import java.util.ArrayList; +import java.util.Comparator; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.concurrent.ConcurrentSkipListSet; import javax.ws.rs.core.Context; @@ -60,8 +62,8 @@ public final class AhcClientHandler implements ClientHandler { private final AhcConfig config; private final AhcRequestWriter requestWriter = new AhcRequestWriter(); - - private final List cookies = new ArrayList(); + //ConcurrentSkipListSet can be iterated with and modified in a thread safe manner + private final ConcurrentSkipListSet cookies = new ConcurrentSkipListSet(new CookieComparator()); @Context private MessageBodyWorkers workers; @@ -146,30 +148,37 @@ public ClientResponse handle(final ClientRequest cr) private void applyResponseCookies(final List responseCookies) { if (responseCookies != null) { for (final Cookie rc : responseCookies) { - // remove existing cookie - final Iterator it = cookies.iterator(); - while (it.hasNext()) { - final Cookie c = it.next(); - if (isSame(rc, c)) { - it.remove(); - break; - } - } - // add new cookie + // add or replace new cookie cookies.add(rc); } } } - private boolean isSame(final Cookie c, final Cookie o) { - return isEquals(c.getDomain(), o.getDomain()) && - isEquals(c.getPath(), o.getPath()) && - isEquals(c.getName(), o.getName()); + public final static class CookieComparator implements Comparator{ + + @Override + public int compare(Cookie o1, Cookie o2) { + if (o1 == o2) return 0; + if (o1 == null) return -1; + if (o2 == null) return 1; + int retval = 0; + retval = compareStrings(o1.getDomain(),o2.getDomain()); + if (retval != 0) return retval; + retval = compareStrings(o1.getPath(),o2.getPath()); + if (retval != 0) return retval; + retval = compareStrings(o1.getName(),o2.getName()); + return retval; + } + public int compareStrings(String o1, String o2) { + if (o1 == o2) return 0; + if (o1 == null) return -1; + if (o2 == null) return 1; + return o1.compareTo(o2); + + } } + - private boolean isEquals(final Object o, final Object o2) { - return (o == null && o2 == null) || o != null && o.equals(o2); - } /** * Check if a body needs to be constructed based on a method's name. diff --git a/src/test/java/org/sonatype/spice/jersey/client/ahc/tests/tests/CookieTest.java b/src/test/java/org/sonatype/spice/jersey/client/ahc/tests/tests/CookieTest.java index c0cbcf3..240f8b7 100644 --- a/src/test/java/org/sonatype/spice/jersey/client/ahc/tests/tests/CookieTest.java +++ b/src/test/java/org/sonatype/spice/jersey/client/ahc/tests/tests/CookieTest.java @@ -40,10 +40,15 @@ package org.sonatype.spice.jersey.client.ahc.tests.tests; +import java.util.concurrent.CountDownLatch; + import com.sun.jersey.api.client.WebResource; import com.sun.jersey.api.container.filter.LoggingFilter; import com.sun.jersey.api.core.DefaultResourceConfig; import com.sun.jersey.api.core.ResourceConfig; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.sonatype.spice.jersey.client.ahc.AhcHttpClient; import org.sonatype.spice.jersey.client.ahc.config.DefaultAhcConfig; @@ -56,6 +61,7 @@ * @author Paul.Sandoz@Sun.Com */ public class CookieTest extends AbstractGrizzlyServerTester { + private final static Logger log = LoggerFactory.getLogger(CookieTest.class); @Path("/") public static class CookieResource { @GET @@ -123,4 +129,56 @@ public void testSessionCookie() { assertEquals("wo-cookie", r.post(String.class)); assertEquals("value", r.get(String.class)); } + + public void testCookieThreading() throws Exception { + ResourceConfig rc = new DefaultResourceConfig(CookieResource.class); + rc.getProperties().put(ResourceConfig.PROPERTY_CONTAINER_REQUEST_FILTERS, + LoggingFilter.class.getName()); + startServer(rc); + + DefaultAhcConfig config = new DefaultAhcConfig(); + AhcHttpClient c = AhcHttpClient.create(config); + + final WebResource r = c.resource(getUri().build()); + // Set first cookie + assertEquals("NO-COOKIE", r.get(String.class)); + CountDownLatch latch = new CountDownLatch(2); + // Test concurrent requests + CookieThreadTest a = new CookieThreadTest(r, latch); + CookieThreadTest b = new CookieThreadTest(r, latch); + a.start(); + b.start(); + latch.await(); + if (a.e != null) { + fail("Unexpected Exception " + a.e); + } + if (b.e != null) { + fail("Unexpected Exception " + a.e); + } + } + public static class CookieThreadTest extends Thread { + private WebResource r; + private CountDownLatch latch; + Exception e; + public CookieThreadTest(WebResource r, CountDownLatch latch) { + this.r =r; + this.latch = latch; + } + @Override + public void run() { + try { + for (int i=0; i<1000; i++) { + assertEquals("value", r.get(String.class)); + } + } catch (RuntimeException e) { + log.error("Threading error",e); + this.e = e; + throw e; + } finally { + latch.countDown(); + } + } + } + + } \ No newline at end of file From 28c0e2ecfe92182c8de2b3176f547f1809c64e02 Mon Sep 17 00:00:00 2001 From: rbarker Date: Tue, 1 Jul 2014 14:03:33 -0700 Subject: [PATCH 2/2] update to java 6 for java.util.concurrent.ConcurrentSkipListSet --- pom.xml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pom.xml b/pom.xml index 84f32ea..fc6ba5f 100644 --- a/pom.xml +++ b/pom.xml @@ -138,13 +138,13 @@ org.codehaus.mojo.signature - java15 + java16 1.0 - check-java-1.5-compat + check-java-1.6-compat process-classes check @@ -195,7 +195,7 @@ 2.0.9 - 1.5 + 1.6