[Cronet] In JavaUrlRequest avoid thread-unsafe access to HttpURLConnection

components/cronet/android/java/src/org/chromium/net/impl/JavaUrlRequest.java

Index: components/cronet/android/java/src/org/chromium/net/impl/JavaUrlRequest.java
diff --git a/components/cronet/android/java/src/org/chromium/net/impl/JavaUrlRequest.java b/components/cronet/android/java/src/org/chromium/net/impl/JavaUrlRequest.java
index 2b81e3eb760690ab8bc7355d21783d2eb735070e..051405cf71019ca54f1e6fbf9d0cbc46a99c1fc6 100644
--- a/components/cronet/android/java/src/org/chromium/net/impl/JavaUrlRequest.java
+++ b/components/cronet/android/java/src/org/chromium/net/impl/JavaUrlRequest.java
@@ -30,6 +30,7 @@ import java.nio.channels.WritableByteChannel;
 import java.util.AbstractMap.SimpleEntry;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.TreeMap;
@@ -126,6 +127,45 @@ final class JavaUrlRequest extends UrlRequestBase {
         CANCELLED,
     }
 
+    // Executor that runs one task at a time on an underlying Executor.
+    private class SeriaizingExecutor implements Executor {
+        private final Executor mUnderlyingExecutor;
+        // Queue of tasks to run.  First element is the task currently executing.
+        // Task processing loop is running if queue is not empty.  Synchronized on itself.
+        private final List<Runnable> mTaskQueue = new LinkedList<>();

[Charles, 2017/01/07 03:48:06]

Also @GuardedBy

[Charles, 2017/01/07 03:48:06]

Use an ArrayDeque instead - faster, offers methods named for queues, and
allocates less memory.

[pauljensen, 2017/01/11 15:29:58]

Done.

[pauljensen, 2017/01/11 15:29:58]

Done.

+
+        public SeriaizingExecutor(Executor underlyingExecutor) {
+            mUnderlyingExecutor = underlyingExecutor;
+        }
+
+        @Override
+        public void execute(final Runnable command) {
+            synchronized (mTaskQueue) {
+                mTaskQueue.add(command);
+                // Task processing loop is already running if there are other items in mTaskQueue.
+                if (mTaskQueue.size() > 1) {
+                    return;
+                }
+            }
+            mUnderlyingExecutor.execute(new Runnable() {
+                @Override
+                public void run() {
+                    Runnable task = command;
+                    while (true) {
+                        task.run();

[Charles, 2017/01/07 03:48:06]

If this throws an exception, other submitted tasks won't run until another task
is submitted - you could fix that by changing this code to submit a new runnable
to the underlying executor for each runnable. 

The reason this isn't done in the Guava implementation is that someone could
wrap a direct executor, and that would cause stack overflow. That's not a risk
here, because we control the executor we're wrapping.

[pauljensen, 2017/01/11 15:29:58]

Done, note that the Runnables can only be submitted after each task has run.

+                        synchronized (mTaskQueue) {
+                            mTaskQueue.remove(0);
+                            if (mTaskQueue.isEmpty()) {
+                                return;
+                            }
+                            task = mTaskQueue.get(0);
+                        }
+                    }
+                }
+            });
+        };
+    }
+
     /**
      * @param executor The executor used for reading and writing from sockets
      * @param userExecutor The executor used to dispatch to {@code callback}
@@ -148,7 +188,7 @@ final class JavaUrlRequest extends UrlRequestBase {
         this.mAllowDirectExecutor = allowDirectExecutor;
         this.mCallbackAsync = new AsyncUrlRequestCallback(callback, userExecutor);
         this.mTrafficStatsTag = TrafficStats.getThreadStatsTag();
-        this.mExecutor = new Executor() {
+        this.mExecutor = new SeriaizingExecutor(new Executor() {
             @Override
             public void execute(final Runnable command) {
                 executor.execute(new Runnable() {
@@ -164,7 +204,7 @@ final class JavaUrlRequest extends UrlRequestBase {
                     }
                 });
             }
-        };
+        });
         this.mCurrentUrl = url;
         this.mUserAgent = userAgent;
     }