You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2012/12/04 22:37:04 UTC
svn commit: r1417199 -
/tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java
Author: markt
Date: Tue Dec 4 21:37:03 2012
New Revision: 1417199
URL: http://svn.apache.org/viewvc?rev=1417199&view=rev
Log:
Fix FindBugs warnings
volatile int -> AtomicIntger so operations are actually atomic
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=1417199&r1=1417198&r2=1417199&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java Tue Dec 4 21:37:03 2012
@@ -25,6 +25,7 @@ import java.util.Iterator;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.Executor;
import java.util.concurrent.RejectedExecutionException;
+import java.util.concurrent.atomic.AtomicInteger;
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
@@ -1134,7 +1135,7 @@ public class AprEndpoint extends Abstrac
private int[] addSocketTimeout;
private int[] addSocketFlags;
- private volatile int addCount = 0;
+ private AtomicInteger addCount = new AtomicInteger(0);
private boolean comet = true;
@@ -1167,7 +1168,7 @@ public class AprEndpoint extends Abstrac
addSocket = new long[size];
addSocketTimeout = new int[size];
addSocketFlags = new int[size];
- addCount = 0;
+ addCount.set(0);
}
/**
@@ -1176,7 +1177,7 @@ public class AprEndpoint extends Abstrac
@Override
public void destroy() {
// Close all sockets in the add queue
- for (int i = 0; i < addCount; i++) {
+ for (int i = 0; i < addCount.get(); i++) {
if (comet) {
processSocket(addSocket[i], SocketStatus.STOP);
} else {
@@ -1187,7 +1188,7 @@ public class AprEndpoint extends Abstrac
closePollset(connectionPollset);
Pool.destroy(pool);
keepAliveCount = 0;
- addCount = 0;
+ addCount.set(0);
try {
while (this.isAlive()) {
this.interrupt();
@@ -1231,7 +1232,7 @@ public class AprEndpoint extends Abstrac
synchronized (this) {
// Add socket to the list. Newly added sockets will wait
// at most for pollTime before being polled
- if (addCount >= addSocket.length) {
+ if (addCount.get() >= addSocket.length) {
// Can't do anything: close the socket right away
if (comet) {
processSocket(socket, SocketStatus.ERROR);
@@ -1240,10 +1241,10 @@ public class AprEndpoint extends Abstrac
}
return;
}
- addSocket[addCount] = socket;
- addSocketTimeout[addCount] = timeout;
- addSocketFlags[addCount] = flags;
- addCount++;
+ addSocket[addCount.get()] = socket;
+ addSocketTimeout[addCount.get()] = timeout;
+ addSocketFlags[addCount.get()] = flags;
+ addCount.incrementAndGet();
// TODO: interrupt poll ?
this.notify();
}
@@ -1271,9 +1272,9 @@ public class AprEndpoint extends Abstrac
if (!running) {
break;
}
- if (keepAliveCount < 1 && addCount < 1) {
+ if (keepAliveCount < 1 && addCount.get() < 1) {
synchronized (this) {
- while (keepAliveCount < 1 && addCount < 1 && running) {
+ while (keepAliveCount < 1 && addCount.get() < 1 && running) {
// Reset maintain time.
maintainTime = 0;
try {
@@ -1290,11 +1291,11 @@ public class AprEndpoint extends Abstrac
}
try {
// Add sockets which are waiting to the poller
- if (addCount > 0) {
+ if (addCount.get() > 0) {
synchronized (this) {
int successCount = 0;
try {
- for (int i = (addCount - 1); i >= 0; i--) {
+ for (int i = (addCount.get() - 1); i >= 0; i--) {
int timeout = addSocketTimeout[i];
if (timeout > 0) {
// Convert milliseconds to microseconds
@@ -1316,7 +1317,7 @@ public class AprEndpoint extends Abstrac
}
} finally {
keepAliveCount += successCount;
- addCount = 0;
+ addCount.set(0);
}
}
}
@@ -1437,11 +1438,11 @@ public class AprEndpoint extends Abstrac
protected long[] desc;
protected HashMap<Long, SendfileData> sendfileData;
- protected volatile int sendfileCount;
- public int getSendfileCount() { return sendfileCount; }
+ protected AtomicInteger sendfileCount = new AtomicInteger(0);
+ public int getSendfileCount() { return sendfileCount.get(); }
protected ArrayList<SendfileData> addS;
- protected volatile int addCount;
+ protected AtomicInteger addCount = new AtomicInteger(0);
/**
* Create the sendfile poller. With some versions of APR, the maximum poller size will
@@ -1462,7 +1463,7 @@ public class AprEndpoint extends Abstrac
desc = new long[size * 2];
sendfileData = new HashMap<>(size);
addS = new ArrayList<>();
- addCount = 0;
+ addCount.set(0);
}
/**
@@ -1471,7 +1472,7 @@ public class AprEndpoint extends Abstrac
@Override
public void destroy() {
// Close any socket remaining in the add queue
- addCount = 0;
+ addCount.set(0);
for (int i = (addS.size() - 1); i >= 0; i--) {
SendfileData data = addS.get(i);
destroySocket(data.socket);
@@ -1559,7 +1560,7 @@ public class AprEndpoint extends Abstrac
// at most for pollTime before being polled
synchronized (this) {
addS.add(data);
- addCount++;
+ addCount.incrementAndGet();
this.notify();
}
return false;
@@ -1573,7 +1574,7 @@ public class AprEndpoint extends Abstrac
protected void remove(SendfileData data) {
int rv = Poll.remove(sendfilePollset, data.socket);
if (rv == Status.APR_SUCCESS) {
- sendfileCount--;
+ sendfileCount.decrementAndGet();
}
sendfileData.remove(Long.valueOf(data.socket));
}
@@ -1601,9 +1602,9 @@ public class AprEndpoint extends Abstrac
if (!running) {
break;
}
- if (sendfileCount < 1 && addCount < 1) {
+ if (sendfileCount.get() < 1 && addCount.get() < 1) {
synchronized (this) {
- while (sendfileCount < 1 && addS.size() < 1 && running) {
+ while (sendfileCount.get() < 1 && addS.size() < 1 && running) {
// Reset maintain time.
maintainTime = 0;
try {
@@ -1620,7 +1621,7 @@ public class AprEndpoint extends Abstrac
}
try {
// Add socket to the poller
- if (addCount > 0) {
+ if (addCount.get() > 0) {
synchronized (this) {
int successCount = 0;
try {
@@ -1637,9 +1638,9 @@ public class AprEndpoint extends Abstrac
}
}
} finally {
- sendfileCount += successCount;
+ sendfileCount.addAndGet(successCount);
addS.clear();
- addCount = 0;
+ addCount.set(0);
}
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1417199 - /tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java
Posted by Konstantin Kolinko <kn...@gmail.com>.
2012/12/5 <ma...@apache.org>:
> Author: markt
> Date: Tue Dec 4 21:37:03 2012
> New Revision: 1417199
>
> URL: http://svn.apache.org/viewvc?rev=1417199&view=rev
> Log:
> Fix FindBugs warnings
> volatile int -> AtomicIntger so operations are actually atomic
>
> 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=1417199&r1=1417198&r2=1417199&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java (original)
> +++ tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java Tue Dec 4 21:37:03 2012
> @@ -25,6 +25,7 @@ import java.util.Iterator;
> import java.util.concurrent.ConcurrentLinkedQueue;
> import java.util.concurrent.Executor;
> import java.util.concurrent.RejectedExecutionException;
> +import java.util.concurrent.atomic.AtomicInteger;
>
> import org.apache.juli.logging.Log;
> import org.apache.juli.logging.LogFactory;
> @@ -1134,7 +1135,7 @@ public class AprEndpoint extends Abstrac
> private int[] addSocketTimeout;
> private int[] addSocketFlags;
>
> - private volatile int addCount = 0;
> + private AtomicInteger addCount = new AtomicInteger(0);
>
> private boolean comet = true;
>
> @@ -1167,7 +1168,7 @@ public class AprEndpoint extends Abstrac
> addSocket = new long[size];
> addSocketTimeout = new int[size];
> addSocketFlags = new int[size];
> - addCount = 0;
> + addCount.set(0);
> }
>
> /**
> @@ -1176,7 +1177,7 @@ public class AprEndpoint extends Abstrac
> @Override
> public void destroy() {
> // Close all sockets in the add queue
> - for (int i = 0; i < addCount; i++) {
> + for (int i = 0; i < addCount.get(); i++) {
> if (comet) {
> processSocket(addSocket[i], SocketStatus.STOP);
Using an atomic "addCount" while addSocket array is not protected by
synchronization is slightly weird. Both of them are supposed to be in
sync with each other.
> } else {
> @@ -1187,7 +1188,7 @@ public class AprEndpoint extends Abstrac
> closePollset(connectionPollset);
> Pool.destroy(pool);
> keepAliveCount = 0;
> - addCount = 0;
> + addCount.set(0);
> try {
> while (this.isAlive()) {
> this.interrupt();
> @@ -1231,7 +1232,7 @@ public class AprEndpoint extends Abstrac
> synchronized (this) {
> // Add socket to the list. Newly added sockets will wait
> // at most for pollTime before being polled
> - if (addCount >= addSocket.length) {
> + if (addCount.get() >= addSocket.length) {
> // Can't do anything: close the socket right away
> if (comet) {
> processSocket(socket, SocketStatus.ERROR);
> @@ -1240,10 +1241,10 @@ public class AprEndpoint extends Abstrac
> }
> return;
> }
> - addSocket[addCount] = socket;
> - addSocketTimeout[addCount] = timeout;
> - addSocketFlags[addCount] = flags;
> - addCount++;
> + addSocket[addCount.get()] = socket;
> + addSocketTimeout[addCount.get()] = timeout;
> + addSocketFlags[addCount.get()] = flags;
> + addCount.incrementAndGet();
> // TODO: interrupt poll ?
> this.notify();
> }
> @@ -1271,9 +1272,9 @@ public class AprEndpoint extends Abstrac
> if (!running) {
> break;
> }
> - if (keepAliveCount < 1 && addCount < 1) {
> + if (keepAliveCount < 1 && addCount.get() < 1) {
> synchronized (this) {
> - while (keepAliveCount < 1 && addCount < 1 && running) {
> + while (keepAliveCount < 1 && addCount.get() < 1 && running) {
> // Reset maintain time.
> maintainTime = 0;
> try {
> @@ -1290,11 +1291,11 @@ public class AprEndpoint extends Abstrac
> }
> try {
> // Add sockets which are waiting to the poller
> - if (addCount > 0) {
> + if (addCount.get() > 0) {
> synchronized (this) {
> int successCount = 0;
> try {
> - for (int i = (addCount - 1); i >= 0; i--) {
> + for (int i = (addCount.get() - 1); i >= 0; i--) {
> int timeout = addSocketTimeout[i];
> if (timeout > 0) {
> // Convert milliseconds to microseconds
> @@ -1316,7 +1317,7 @@ public class AprEndpoint extends Abstrac
> }
> } finally {
> keepAliveCount += successCount;
> - addCount = 0;
> + addCount.set(0);
> }
> }
> }
> @@ -1437,11 +1438,11 @@ public class AprEndpoint extends Abstrac
> protected long[] desc;
> protected HashMap<Long, SendfileData> sendfileData;
>
> - protected volatile int sendfileCount;
> - public int getSendfileCount() { return sendfileCount; }
> + protected AtomicInteger sendfileCount = new AtomicInteger(0);
> + public int getSendfileCount() { return sendfileCount.get(); }
>
> protected ArrayList<SendfileData> addS;
> - protected volatile int addCount;
> + protected AtomicInteger addCount = new AtomicInteger(0);
>
> /**
> * Create the sendfile poller. With some versions of APR, the maximum poller size will
> @@ -1462,7 +1463,7 @@ public class AprEndpoint extends Abstrac
> desc = new long[size * 2];
> sendfileData = new HashMap<>(size);
> addS = new ArrayList<>();
> - addCount = 0;
> + addCount.set(0);
> }
>
> /**
> @@ -1471,7 +1472,7 @@ public class AprEndpoint extends Abstrac
> @Override
> public void destroy() {
> // Close any socket remaining in the add queue
> - addCount = 0;
> + addCount.set(0);
> for (int i = (addS.size() - 1); i >= 0; i--) {
> SendfileData data = addS.get(i);
> destroySocket(data.socket);
> @@ -1559,7 +1560,7 @@ public class AprEndpoint extends Abstrac
> // at most for pollTime before being polled
> synchronized (this) {
> addS.add(data);
> - addCount++;
> + addCount.incrementAndGet();
> this.notify();
> }
> return false;
> @@ -1573,7 +1574,7 @@ public class AprEndpoint extends Abstrac
> protected void remove(SendfileData data) {
> int rv = Poll.remove(sendfilePollset, data.socket);
> if (rv == Status.APR_SUCCESS) {
> - sendfileCount--;
> + sendfileCount.decrementAndGet();
> }
> sendfileData.remove(Long.valueOf(data.socket));
> }
> @@ -1601,9 +1602,9 @@ public class AprEndpoint extends Abstrac
> if (!running) {
> break;
> }
> - if (sendfileCount < 1 && addCount < 1) {
> + if (sendfileCount.get() < 1 && addCount.get() < 1) {
> synchronized (this) {
> - while (sendfileCount < 1 && addS.size() < 1 && running) {
> + while (sendfileCount.get() < 1 && addS.size() < 1 && running) {
> // Reset maintain time.
> maintainTime = 0;
> try {
> @@ -1620,7 +1621,7 @@ public class AprEndpoint extends Abstrac
> }
> try {
> // Add socket to the poller
> - if (addCount > 0) {
> + if (addCount.get() > 0) {
> synchronized (this) {
> int successCount = 0;
> try {
> @@ -1637,9 +1638,9 @@ public class AprEndpoint extends Abstrac
> }
> }
> } finally {
> - sendfileCount += successCount;
> + sendfileCount.addAndGet(successCount);
> addS.clear();
> - addCount = 0;
> + addCount.set(0);
> }
> }
> }
>
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org