diff options
author | Tor Brede Vekterli <vekterli@verizonmedia.com> | 2020-05-19 15:31:06 +0200 |
---|---|---|
committer | Tor Brede Vekterli <vekterli@verizonmedia.com> | 2020-05-19 15:31:06 +0200 |
commit | a9301a966c228e55c38371df6a917db568c48e39 (patch) | |
tree | c6988f649a8e6b392bff33f1fc80116e1319ea82 /documentapi | |
parent | 69b3bbee1cd90ec5836d5ef1fd06e80072acd450 (diff) |
Avoid data race from pending visitor send task
It's currently possible for the visitor session to complete
even if there's a SendCreateVisitors task scheduled. This
will usually happen if there's been an error or if a sufficient
number of documents has been retrieved, triggering an early
exit of the session. In this case we must ensure that we do
not mutate any shared data structures from the send task when
it finally executes, as they may be read concurrently by the
session client thread.
The `done` variable is written under the same mutex as that held
by the send task, so visibility is guaranteed.
Also add `synchronized` to the binary serialization method for
`ProgressToken` to match the existing non-binary serialization
methods. This should not be required with the main race condition
fix, but is included for completion. Shall not break ABI compatibility.
Diffstat (limited to 'documentapi')
3 files changed, 5 insertions, 2 deletions
diff --git a/documentapi/abi-spec.json b/documentapi/abi-spec.json index 3092c352ee4..71ef49fcd1b 100644 --- a/documentapi/abi-spec.json +++ b/documentapi/abi-spec.json @@ -293,7 +293,7 @@ "public void <init>(int)", "public void <init>(java.lang.String)", "public void <init>(byte[])", - "public byte[] serialize()", + "public synchronized byte[] serialize()", "public java.lang.String serializeToString()", "public static com.yahoo.documentapi.ProgressToken fromSerializedString(java.lang.String)", "public void addFailedBucket(com.yahoo.document.BucketId, com.yahoo.document.BucketId, java.lang.String)", diff --git a/documentapi/src/main/java/com/yahoo/documentapi/ProgressToken.java b/documentapi/src/main/java/com/yahoo/documentapi/ProgressToken.java index 7e579a3ae6a..540b92461e6 100644 --- a/documentapi/src/main/java/com/yahoo/documentapi/ProgressToken.java +++ b/documentapi/src/main/java/com/yahoo/documentapi/ProgressToken.java @@ -226,7 +226,7 @@ public class ProgressToken { } } - public byte[] serialize() { + public synchronized byte[] serialize() { BufferSerializer out = new BufferSerializer(new GrowableByteBuffer()); out.putInt(null, distributionBits); out.putLong(null, bucketCursor); diff --git a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusVisitorSession.java b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusVisitorSession.java index 1ad84cb0b25..00a68c99500 100755 --- a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusVisitorSession.java +++ b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusVisitorSession.java @@ -643,6 +643,9 @@ public class MessageBusVisitorSession implements VisitorSession { synchronized (progress.getToken()) { try { scheduledSendCreateVisitors = false; + if (done) { + return; // Session already closed; we must not touch anything else. + } while (progress.getIterator().hasNext()) { VisitorIterator.BucketProgress bucket = progress.getIterator().getNext(); Result result = sender.send(createMessage(bucket)); |