Skip to content

Commit 73911eb

Browse files
pratt4prat
and
prat
authored
Fix NPE race in NettyResponseFuture.cancel (#2042) (#2088)
Fixes #2042 This is a typical TOCTOU (time-of-check/time-of-use) race https://en.wikipedia.org/wiki/Time-of-check_to_time-of-use. The NPE was occurring because the channel field could be set to null by another thread between the check and its use: if (channel != null) { // time-of-check Channels.setDiscard(channel); // time-of-use Channels.silentlyCloseChannel(channel); } By copying channel into a local variable in one atomic read, we ensure that—even if another thread changes the field—the local reference remains valid. P.S. It is hard to write a deterministic test that fails consistently, so this PR only includes the code fix. --------- Co-authored-by: prat <[email protected]>
1 parent 14ee30a commit 73911eb

File tree

1 file changed

+4
-4
lines changed

1 file changed

+4
-4
lines changed

client/src/main/java/org/asynchttpclient/netty/NettyResponseFuture.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -187,10 +187,10 @@ public boolean cancel(boolean force) {
187187
return false;
188188
}
189189

190-
// cancel could happen before channel was attached
191-
if (channel != null) {
192-
Channels.setDiscard(channel);
193-
Channels.silentlyCloseChannel(channel);
190+
final Channel ch = channel; //atomic read, so that it won't end up in TOCTOU
191+
if (ch != null) {
192+
Channels.setDiscard(ch);
193+
Channels.silentlyCloseChannel(ch);
194194
}
195195

196196
if (ON_THROWABLE_CALLED_FIELD.getAndSet(this, 1) == 0) {

0 commit comments

Comments
 (0)