Commit 91f925c1 authored by Caleb Raitto's avatar Caleb Raitto Committed by Commit Bot

Clarify threading of DirectPreventingExecutor.

- Clarify why the read of runnable.mExecutedInline is safe.
- Fix bad wrapping in an existing comment.
- Clarify why writing "runnable.mCallingThread = null;" is safe.

Change-Id: Iefb87004f106e400dfa02a61fa1d6f1120d2fbe4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1743278Reviewed-by: default avatarPaul Jensen <pauljensen@chromium.org>
Commit-Queue: Caleb Raitto <caraitto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#688177}
parent e76a9c28
...@@ -77,14 +77,21 @@ public final class JavaUrlRequestUtils { ...@@ -77,14 +77,21 @@ public final class JavaUrlRequestUtils {
Thread currentThread = Thread.currentThread(); Thread currentThread = Thread.currentThread();
InlineCheckingRunnable runnable = new InlineCheckingRunnable(command, currentThread); InlineCheckingRunnable runnable = new InlineCheckingRunnable(command, currentThread);
mDelegate.execute(runnable); mDelegate.execute(runnable);
// This next read doesn't require synchronization; only the current thread could have
// written to runnable.mExecutedInline.
if (runnable.mExecutedInline != null) { if (runnable.mExecutedInline != null) {
throw runnable.mExecutedInline; throw runnable.mExecutedInline;
} else { } else {
// It's possible that this method is being called on an executor, and the runnable // It's possible that this method is being called on an executor, and the runnable
// that // that was just queued will run on this thread after the current runnable returns.
// was just queued will run on this thread after the current runnable returns. By // By nulling out the mCallingThread field, the InlineCheckingRunnable's current
// nulling out the mCallingThread field, the InlineCheckingRunnable's current thread // thread comparison will not fire.
// comparison will not fire. //
// Java reference assignment is always atomic (no tearing, even on 64-bit VMs, see
// JLS 17.7), but other threads aren't guaranteed to ever see updates without
// something like locking, volatile, or AtomicReferences. We're ok in
// this instance, since this write only needs to be seen in the case that
// InlineCheckingRunnable.run() runs on the same thread as this execute() method.
runnable.mCallingThread = null; runnable.mCallingThread = null;
} }
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment