Skip to content

Commit 2b062fc

Browse files
authored
Fixed a case where received messages may not be released properly (#11)
Fixed a case where received messages may not be released properly if a lot of requests are timing out before their responses are received.
1 parent aa567a4 commit 2b062fc

File tree

3 files changed

+51
-1
lines changed

3 files changed

+51
-1
lines changed

driver/src/main/java/oracle/nosql/driver/httpclient/HttpClient.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77

88
package oracle.nosql.driver.httpclient;
99

10+
import static oracle.nosql.driver.util.LogUtil.isFineEnabled;
11+
import static oracle.nosql.driver.util.LogUtil.logFine;
1012
import static oracle.nosql.driver.util.LogUtil.logInfo;
1113
import static oracle.nosql.driver.util.LogUtil.logWarning;
1214

@@ -309,6 +311,18 @@ public Channel getChannel(int timeoutMs)
309311
* Ensure that the channel is in good shape
310312
*/
311313
if (fut.isSuccess() && retChan.isActive()) {
314+
/*
315+
* Clear out any previous state. The channel should not
316+
* have any state associated with it, but this code is here
317+
* just in case it does.
318+
*/
319+
if (retChan.attr(STATE_KEY).get() != null) {
320+
if (isFineEnabled(logger)) {
321+
logFine(logger, "HttpClient acquired a channel with " +
322+
"a still-active state: clearing.");
323+
}
324+
retChan.attr(STATE_KEY).set(null);
325+
}
312326
return retChan;
313327
}
314328
logInfo(logger,
@@ -319,6 +333,9 @@ public Channel getChannel(int timeoutMs)
319333
}
320334

321335
public void releaseChannel(Channel channel) {
336+
/* Clear any response handler state from channel before releasing it */
337+
channel.attr(STATE_KEY).set(null);
338+
322339
/*
323340
* If channel is not healthy/active it will be closed and removed
324341
* from the pool. Don't wait for completion.

driver/src/main/java/oracle/nosql/driver/httpclient/HttpClientHandler.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77

88
package oracle.nosql.driver.httpclient;
99

10+
import static oracle.nosql.driver.util.HttpConstants.REQUEST_ID_HEADER;
11+
import static oracle.nosql.driver.util.LogUtil.isFineEnabled;
12+
import static oracle.nosql.driver.util.LogUtil.logFine;
1013
import static oracle.nosql.driver.util.LogUtil.logWarning;
1114

1215
import java.io.IOException;
@@ -43,6 +46,26 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) {
4346

4447
if (msg instanceof FullHttpResponse) {
4548
FullHttpResponse fhr = (FullHttpResponse) msg;
49+
50+
if (state == null) {
51+
/*
52+
* This message came in after the client was done processing
53+
* a request in a different thread.
54+
* The client may have timed out waiting for this message.
55+
* Discard the message by releasing it and not calling receive().
56+
*/
57+
if (isFineEnabled(logger)) {
58+
String requestId = fhr.headers().get(REQUEST_ID_HEADER);
59+
if (requestId == null) {
60+
requestId = "(none)";
61+
}
62+
logFine(logger, "Discarding message with no response " +
63+
"handler. requestId=" + requestId);
64+
}
65+
fhr.release();
66+
return;
67+
}
68+
4669
state.setResponse(fhr);
4770

4871
/*

driver/src/main/java/oracle/nosql/driver/httpclient/ResponseHandler.java

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,14 +180,24 @@ void receive(RequestState requestState) {
180180
String resReqId = requestState.getHeaders().get(REQUEST_ID_HEADER);
181181
if (resReqId == null || !resReqId.equals(requestId)) {
182182
logFine(logger,
183-
"Discards unpaired response: expect for request " +
183+
"Discarding unpaired response: expect for request " +
184184
requestId + ", but for request " + resReqId);
185185
if (requestState.getResponse() != null) {
186186
ReferenceCountUtil.release(requestState.getResponse());
187187
}
188188
return;
189189
}
190190
}
191+
192+
/*
193+
* We got a valid message: don't accept any more for this handler.
194+
* This logic may change if we enable full async and allow multiple
195+
* messages to be processed by the same channel for the same client.
196+
* This clears the response handler from this channel so that any
197+
* additional messages on this channel will be properly discarded.
198+
*/
199+
channel.attr(HttpClient.STATE_KEY).set(null);
200+
191201
state = requestState;
192202
try {
193203
responseReceived(state.getStatus(),

0 commit comments

Comments
 (0)