readBufferBytesAllocated TNonblockingServer.java should be AtomicLong to fix FD
leakage and general server malfunction
----------------------------------------------------------------------------------------------------------------------
Key: THRIFT-1190
URL: https://issues.apache.org/jira/browse/THRIFT-1190
Project: Thrift
Issue Type: Bug
Components: Java - Library
Affects Versions: 0.6
Environment: Any
Reporter: Tom May
We're having a problem using THsHaServer where the server doesn't suddenly
stops closing close connections and runs out of file descriptors.
It looks like the problem is in TNonblockingServer.java. The variable "long
readBufferBytesAllocated" is shared between threads but isn't synchronized.
Intermittently, its value can get out of whack due to races and cause this if
statement to always execute:
// if this frame will push us over the memory limit, then return.
// with luck, more memory will free up the next time around.
if (readBufferBytesAllocated.get() + frameSize >
MAX_READ_BUFFER_BYTES) {
return true;
}
We started having this problem when we set MAX_READ_BUFFER_BYTES to a somewhat
small size, which unmasked the issue.
I don't see anywhere here to attach a patch so here it is:
Index: lib/java/src/org/apache/thrift/server/TNonblockingServer.java
===================================================================
--- lib/java/src/org/apache/thrift/server/TNonblockingServer.java
(revision 1129978)
+++ lib/java/src/org/apache/thrift/server/TNonblockingServer.java
(working copy)
@@ -28,6 +28,7 @@
import java.util.HashSet;
import java.util.Iterator;
import java.util.Set;
+import java.util.concurrent.atomic.AtomicLong;
import org.apache.thrift.TByteArrayOutputStream;
import org.apache.thrift.TException;
@@ -87,7 +88,7 @@
/**
* How many bytes are currently allocated to read buffers.
*/
- private long readBufferBytesAllocated = 0;
+ private final AtomicLong readBufferBytesAllocated = new AtomicLong(0);
public TNonblockingServer(AbstractNonblockingServerArgs args) {
super(args);
@@ -479,12 +480,12 @@
// if this frame will push us over the memory limit, then return.
// with luck, more memory will free up the next time around.
- if (readBufferBytesAllocated + frameSize > MAX_READ_BUFFER_BYTES) {
+ if (readBufferBytesAllocated.get() + frameSize >
MAX_READ_BUFFER_BYTES) {
return true;
}
// increment the amount of memory allocated to read buffers
- readBufferBytesAllocated += frameSize;
+ readBufferBytesAllocated.addAndGet(frameSize);
// reallocate the readbuffer as a frame-sized buffer
buffer_ = ByteBuffer.allocate(frameSize);
@@ -576,7 +577,7 @@
// if we're being closed due to an error, we might have allocated a
// buffer that we need to subtract for our memory accounting.
if (state_ == READING_FRAME || state_ == READ_FRAME_COMPLETE) {
- readBufferBytesAllocated -= buffer_.array().length;
+ readBufferBytesAllocated.addAndGet(-buffer_.array().length);
}
trans_.close();
}
@@ -600,7 +601,7 @@
// our read buffer count. we do this here as well as in close because
// we'd like to free this read memory up as quickly as possible for other
// clients.
- readBufferBytesAllocated -= buffer_.array().length;
+ readBufferBytesAllocated.addAndGet(-buffer_.array().length);
if (response_.len() == 0) {
// go straight to reading again. this was probably an oneway method
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
