[Cronet] Fix race in UploadDataStream.attachToRequest.

components/cronet/android/java/src/org/chromium/net/CronetUrlRequest.java

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.net;
 
 import android.os.SystemClock;
 import android.support.annotation.Nullable;
 import android.util.Log;
 
@@ skipping 192 matching lines @@
                                 mUrlRequestAdapter, header.getKey(), header.getValue())) {
                         throw new IllegalArgumentException(
                                 "Invalid header " + header.getKey() + "=" + header.getValue());
                     }
                 }
                 if (mUploadDataStream != null) {
                     if (!hasContentType) {
                         throw new IllegalArgumentException(
                                 "Requests with upload data must have a Content-Type.");
                     }
-                    mStarted = true;

[mef, 2016/03/10 17:10:39]

This was also a problem as 'isDone' is looking at this flag.

[xunjieli, 2016/03/10 17:29:53]

But if start() is called a second time before the runnable is executed, won't
that make the request to be started a second time?

[mef, 2016/03/10 17:44:05]

Ah, excellent point! Done.

-                    mUploadDataStream.attachToRequest(this, mUrlRequestAdapter, new Runnable() {
+                    mUploadDataStream.postTaskToExecutor(new Runnable() {
                         @Override
                         public void run() {
                             synchronized (mUrlRequestAdapterLock) {
                                 startInternalLocked();
                             }
                         }
                     });
                     return;
                 }
             } catch (RuntimeException e) {
                 // If there's an exception, cleanup and then throw the
                 // exception to the caller.
                 destroyRequestAdapter(false);
                 throw e;
             }
-            mStarted = true;
             startInternalLocked();
         }
     }
 
     @GuardedBy("mUrlRequestAdapterLock")
     private void startInternalLocked() {
+        if (mUrlRequestAdapter == 0) {
+            return;
+        }
+        if (mUploadDataStream != null) {
+            mUploadDataStream.attachToRequest(this, mUrlRequestAdapter);

[mef, 2016/03/10 17:10:39]

attach only if request is still valid.

+        }
         if (mDisableCache) {
             nativeDisableCache(mUrlRequestAdapter);
         }
         if (mRequestMetricsAccumulator != null) {
             mRequestMetricsAccumulator.onRequestStarted();
         }
+        mStarted = true;
         nativeStart(mUrlRequestAdapter);
     }
 
     @Override
     public void followRedirect() {
         synchronized (mUrlRequestAdapterLock) {
             if (!mWaitingOnRedirect) {
                 throw new IllegalStateException("No redirect to follow.");
             }
             mWaitingOnRedirect = false;
@@ skipping 483 matching lines @@
     @NativeClassQualifiedName("CronetURLRequestAdapter")
     private native boolean nativeReadData(long nativePtr, ByteBuffer byteBuffer,
             int position, int capacity);
 
     @NativeClassQualifiedName("CronetURLRequestAdapter")
     private native void nativeDestroy(long nativePtr, boolean sendOnCanceled);
 
     @NativeClassQualifiedName("CronetURLRequestAdapter")
     private native void nativeGetStatus(long nativePtr, UrlRequest.StatusListener listener);
 }