Skip to content

Commit bf284c8

Browse files
committed
Fix a race condition in Deferred.toString()
The `result' field was checked against `null' and then used. However this field can change at any time, so we must first copy it into a local variable to make sure it doesn't become `null' at an incongruous moment, which could cause a `NullPointerException'. This issue is especially likely to occur when a `TimeoutException' is raised roughly at the same time as the `result' field is being updated by a callback. Bug reported by Viral Bajaria.
1 parent 73d611d commit bf284c8

File tree

2 files changed

+10
-8
lines changed

2 files changed

+10
-8
lines changed

THANKS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,4 @@ suggesting improvements, submitting patches or proofreading the code.
55

66
Michael Stack
77
Nick Telford
8+
Viral Bajaria

src/Deferred.java

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1375,13 +1375,14 @@ public String toString() {
13751375
*/
13761376
public String toString() {
13771377
final int state = this.state; // volatile access before reading result.
1378-
String result;
1379-
if (this.result == null) {
1380-
result = "null";
1381-
} else if (this.result instanceof Deferred) { // Nested Deferreds
1382-
result = "Deferred@" + this.result.hashCode(); // are hard to read.
1378+
final Object result = this.result;
1379+
final String str;
1380+
if (result == null) {
1381+
str = "null";
1382+
} else if (result instanceof Deferred) { // Nested Deferreds
1383+
str = "Deferred@" + result.hashCode(); // are hard to read.
13831384
} else {
1384-
result = this.result.toString();
1385+
str = result.toString();
13851386
}
13861387

13871388
// We can't easily estimate how much space we'll need for the callback
@@ -1390,10 +1391,10 @@ public String toString() {
13901391
// If result is a very long string, we may end up wasting some space, but
13911392
// it's not likely to happen and even less likely to be a problem.
13921393
final StringBuilder buf = new StringBuilder((9 + 10 + 7 + 7
1393-
+ result.length()) * 2);
1394+
+ str.length()) * 2);
13941395
buf.append("Deferred@").append(super.hashCode())
13951396
.append("(state=").append(stateString(state))
1396-
.append(", result=").append(result)
1397+
.append(", result=").append(str)
13971398
.append(", callback=");
13981399
synchronized (this) {
13991400
if (callbacks == null || next_callback == last_callback) {

0 commit comments

Comments
 (0)