Author: kkolinko Date: Thu Jun 3 01:33:08 2010 New Revision: 950851 URL: http://svn.apache.org/viewvc?rev=950851&view=rev Log: 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
Modified: tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java Modified: tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java?rev=950851&r1=950850&r2=950851&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java (original) +++ tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java Thu Jun 3 01:33:08 2010 @@ -918,11 +918,11 @@ public class AprEndpoint extends Abstrac 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 Abstrac } } - 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 Abstrac // 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 Abstrac protected long[] desc; protected HashMap<Long, SendfileData> sendfileData; - protected int sendfileCount; + protected volatile int sendfileCount; public int getSendfileCount() { return sendfileCount; } protected ArrayList<SendfileData> 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 Abstrac desc = new long[size * 2]; sendfileData = new HashMap<Long, SendfileData>(size); addS = new ArrayList<SendfileData>(); + addCount = 0; } /** @@ -1220,6 +1229,7 @@ public class AprEndpoint extends Abstrac // 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 Abstrac // 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 Abstrac } } - 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(); } } --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org