[Cronet] Fix two races/leaks in JavaUrlRequest

components/cronet/android/test/javatests/src/org/chromium/net/UploadDataProvidersTest.java

 // Copyright 2016 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.ConditionVariable;
 import android.os.ParcelFileDescriptor;
 import android.os.StrictMode;
 import android.support.test.filters.SmallTest;
@@ skipping 211 matching lines @@
         UploadDataProvider dataProvider =
                 UploadDataProviders.create(uploadDataWithPadding, offset, uploadData.length);
         assertEquals(uploadData.length, dataProvider.getLength());
         builder.setUploadDataProvider(dataProvider, callback.getExecutor());
         UrlRequest urlRequest = builder.build();
         urlRequest.start();
         callback.blockForDone();
         assertEquals(200, callback.mResponseInfo.getHttpStatusCode());
         assertEquals(LOREM, callback.mResponseAsString);
     }
+
+    @SmallTest
+    @Feature({"Cronet"})
+    /**
+     * Open many connections and cancel them right away. This test verifies all internal
+     * sockets and other Closeables are properly closed. See crbug.com/726193. This isn't
+     * really a test of UploadDataProviders but is in this class to piggyback on the
+     * StrictMode enforcement it offers.

[Charles, 2017/05/26 13:58:48]

Should we add that same strictmode checking to CronetTestBase?

[pauljensen, 2017/05/26 18:36:03]

Done.

+     */
+    public void testGzipCancel() throws Exception {
+        String url = NativeTestServer.getFileURL("/gzipped.html");
+        for (int i = 0; i < 100; i++) {
+            TestUrlRequestCallback callback = new TestUrlRequestCallback();
+            callback.setAutoAdvance(false);
+            UrlRequest urlRequest =
+                    mTestFramework.mCronetEngine
+                            .newUrlRequestBuilder(url, callback, callback.getExecutor())
+                            .build();
+            urlRequest.start();
+            urlRequest.cancel();
+        }

[Charles, 2017/05/26 13:58:48]

Shouldn't you block on all these operations completing before letting the test
finish?

[pauljensen, 2017/05/26 18:36:03]

If I block until each UrlRequest is done before starting the next then I never
catch the leak.  If I start them all and then wait for them all to finish, I
only catch the leak ~10% of the time.  In its current form I catch the leak ~70%
of the time.

I think the leak requires a lot of busy threads so that the cancel() happens
before the UrlRequest has made much progress (and set mCurrentUrlConnection and
mResponseChannel).  So this explains why the waiting for each one doesn't catch
the leak.

The other quirk of this is that from teardown() we call
JavaCronetEngine.shutdown() which calls ExecutorService.shutdown() which doesn't
wait for the thread to finish running tasks, and then we call GC looking for
leaks.  We could expose the ExecutorService and then have tests call
awaitTermination() but I think the simple test in its current form is probably
good enough.

+    }
 }