cshannon commented on code in PR #2474:
URL: https://github.com/apache/activemq/pull/2474#discussion_r3823572987
##########
activemq-client/src/main/java/org/apache/activemq/transport/tcp/TcpTransport.java:
##########
@@ -613,26 +613,36 @@ public void stop() throws Exception {
}
protected void initializeStreams() throws Exception {
+ // receiveCounter feeds AbstractInactivityMonitor.readCheck() so only
increment when there is data read
TcpBufferedInputStream buffIn = new
TcpBufferedInputStream(socket.getInputStream(), ioBufferSize) {
@Override
public int read() throws IOException {
- receiveCounter.incrementAndGet();
- return super.read();
+ int result = super.read();
+ if (result >= 0) {
+ receiveCounter.incrementAndGet();
+ }
+ return result;
}
@Override
public int read(byte[] b, int off, int len) throws IOException {
- receiveCounter.incrementAndGet();
- return super.read(b, off, len);
+ int result = super.read(b, off, len);
+ if (result > 0) {
+ receiveCounter.incrementAndGet();
+ }
+ return result;
}
@Override
public long skip(long n) throws IOException {
- receiveCounter.incrementAndGet();
- return super.skip(n);
+ long result = super.skip(n);
+ if (result > 0) {
+ receiveCounter.incrementAndGet();
+ }
+ return result;
}
@Override
protected void fill() throws IOException {
- receiveCounter.incrementAndGet();
super.fill();
+ receiveCounter.incrementAndGet();
Review Comment:
Looks like this is the main fix here, the fill() method gets called over and
over when the soTimeout happens so this prevents incrementing the counter
unless it finishes.
What if we update the fill() method to return the number of bytes read? That
seems more correct.
```java
@Override
protected int fill() throws IOException {
int read = super.fill();
if (read > 0) {
receiveCounter.incrementAndGet();
}
return read;
}
```
The [super
method](https://github.com/apache/activemq/blob/e0dca51c19d9c39e5e63915ff8ceb5ad1a2277cf/activemq-client/src/main/java/org/apache/activemq/transport/tcp/TcpBufferedInputStream.java#L46)
would just become:
```java
protected int fill() throws IOException {
byte[] buffer = internalBuffer;
count = 0;
position = 0;
int n = in.read(buffer, position, buffer.length - position);
if (n > 0) {
count = n + position;
}
return n;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
