From db3b9a7bd4d647cbb1ab9195142df4e8497b6612 Mon Sep 17 00:00:00 2001 From: kkolinko Date: Thu, 3 Jun 2010 01:33:08 +0000 Subject: [PATCH] Prevent possible deadlocks in AprEndpoint.Poller and AprEndpoint.Sendfile, caused by missing Object.notify() wakeup, which is similar to https://issues.apache.org/bugzilla/show_bug.cgi?id=48843 git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@950851 13f79535-47bb-0310-9956-ffa450edef68 --- java/org/apache/tomcat/util/net/AprEndpoint.java | 103 ++++++++++++++--------- 1 file changed, 61 insertions(+), 42 deletions(-) diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java b/java/org/apache/tomcat/util/net/AprEndpoint.java index ca8b05749..c5fd0275d 100644 --- a/java/org/apache/tomcat/util/net/AprEndpoint.java +++ b/java/org/apache/tomcat/util/net/AprEndpoint.java @@ -918,11 +918,11 @@ public class AprEndpoint extends AbstractEndpoint { protected long[] desc; protected long[] addS; - protected int addCount = 0; + protected volatile int addCount = 0; protected boolean comet = true; - protected int keepAliveCount = 0; + protected volatile int keepAliveCount = 0; public int getKeepAliveCount() { return keepAliveCount; } public Poller(boolean comet) { @@ -1038,15 +1038,17 @@ public class AprEndpoint extends AbstractEndpoint { } } - while (keepAliveCount < 1 && addCount < 1) { - // Reset maintain time. - maintainTime = 0; - try { - synchronized (this) { - this.wait(); + if (keepAliveCount < 1 && addCount < 1) { + synchronized (this) { + while (keepAliveCount < 1 && addCount < 1) { + // Reset maintain time. + maintainTime = 0; + try { + this.wait(); + } catch (InterruptedException e) { + // Ignore + } } - } catch (InterruptedException e) { - // Ignore } } @@ -1054,21 +1056,26 @@ public class AprEndpoint extends AbstractEndpoint { // Add sockets which are waiting to the poller if (addCount > 0) { synchronized (this) { - for (int i = (addCount - 1); i >= 0; i--) { - int rv = Poll.add - (serverPollset, addS[i], Poll.APR_POLLIN); - if (rv == Status.APR_SUCCESS) { - keepAliveCount++; - } else { - // Can't do anything: close the socket right away - if (comet) { - processSocket(addS[i], SocketStatus.ERROR); + int successCount = 0; + try { + for (int i = (addCount - 1); i >= 0; i--) { + int rv = Poll.add + (serverPollset, addS[i], Poll.APR_POLLIN); + if (rv == Status.APR_SUCCESS) { + successCount++; } else { - Socket.destroy(addS[i]); + // Can't do anything: close the socket right away + if (comet) { + processSocket(addS[i], SocketStatus.ERROR); + } else { + Socket.destroy(addS[i]); + } } } + } finally { + keepAliveCount += successCount; + addCount = 0; } - addCount = 0; } } @@ -1179,10 +1186,11 @@ public class AprEndpoint extends AbstractEndpoint { protected long[] desc; protected HashMap sendfileData; - protected int sendfileCount; + protected volatile int sendfileCount; public int getSendfileCount() { return sendfileCount; } protected ArrayList addS; + protected volatile int addCount; /** * Create the sendfile poller. With some versions of APR, the maximum poller size will @@ -1203,6 +1211,7 @@ public class AprEndpoint extends AbstractEndpoint { desc = new long[size * 2]; sendfileData = new HashMap(size); addS = new ArrayList(); + addCount = 0; } /** @@ -1220,6 +1229,7 @@ public class AprEndpoint extends AbstractEndpoint { // Ignore } // Close any socket remaining in the add queue + addCount = 0; for (int i = (addS.size() - 1); i >= 0; i--) { SendfileData data = addS.get(i); Socket.destroy(data.socket); @@ -1287,6 +1297,7 @@ public class AprEndpoint extends AbstractEndpoint { // at most for pollTime before being polled synchronized (this) { addS.add(data); + addCount++; this.notify(); } return false; @@ -1324,35 +1335,43 @@ public class AprEndpoint extends AbstractEndpoint { } } - while (sendfileCount < 1 && addS.size() < 1) { - // Reset maintain time. - maintainTime = 0; - try { - synchronized (this) { - this.wait(); + if (sendfileCount < 1 && addCount < 1) { + synchronized (this) { + while (sendfileCount < 1 && addS.size() < 1) { + // Reset maintain time. + maintainTime = 0; + try { + this.wait(); + } catch (InterruptedException e) { + // Ignore + } } - } catch (InterruptedException e) { - // Ignore } } try { // Add socket to the poller - if (addS.size() > 0) { + if (addCount > 0) { synchronized (this) { - for (int i = (addS.size() - 1); i >= 0; i--) { - SendfileData data = addS.get(i); - int rv = Poll.add(sendfilePollset, data.socket, Poll.APR_POLLOUT); - if (rv == Status.APR_SUCCESS) { - sendfileData.put(new Long(data.socket), data); - sendfileCount++; - } else { - log.warn(sm.getString("endpoint.sendfile.addfail", "" + rv, Error.strerror(rv))); - // Can't do anything: close the socket right away - Socket.destroy(data.socket); + int successCount = 0; + try { + for (int i = (addS.size() - 1); i >= 0; i--) { + SendfileData data = addS.get(i); + int rv = Poll.add(sendfilePollset, data.socket, Poll.APR_POLLOUT); + if (rv == Status.APR_SUCCESS) { + sendfileData.put(new Long(data.socket), data); + successCount++; + } else { + log.warn(sm.getString("endpoint.sendfile.addfail", "" + rv, Error.strerror(rv))); + // Can't do anything: close the socket right away + Socket.destroy(data.socket); + } } + } finally { + sendfileCount += successCount; + addS.clear(); + addCount = 0; } - addS.clear(); } } -- 2.11.0