New issue
Advanced search Search tips

Issue 844031 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug

Blocked on:
issue 844045



Sign in to add a comment

[Cronet] Fallback JavaCronetEngine creates huge number of threads

Project Member Reported by mef@chromium.org, May 17 2018

Issue description

JavaCronetEngine is using platform Java stack to implement subset of Cronet API. 

It is most often used if native library cannot be loaded. 

Current implementation doesn't handle big number of simultaneous requests well because its thread pool is not bounded and at worst could have one thread per started request, causing OOM crash.

The internal issue that provides more details is b/79490709.
 
Owner: pauljensen@chromium.org
Status: Started (was: Available)
I'll try fixing.  Looks like our JavaUrlRequest.start() already has a short-circuit if cancel() has been called.

Comment 2 by mef@chromium.org, May 17 2018

Blockedon: 844045

Comment 3 by mef@chromium.org, May 17 2018

Thanks Paul, I've tried (https://chromium-review.googlesource.com/c/chromium/src/+/1055762) to limit the size of the pool, but I ran into issues with 1000 simulataneous exabyte responses and got stuck there.
Project Member

Comment 4 by bugdroid1@chromium.org, May 22 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2771f9828d8b640dbdf79237c0002928627805ba

commit 2771f9828d8b640dbdf79237c0002928627805ba
Author: Paul Jensen <pauljensen@chromium.org>
Date: Tue May 22 14:36:52 2018

[Cronet] Limit JavaCronetEngine thread pool size to avoid OOM crashes

Also add regression test that demonstrates the OOM exception without the fix.
Also fix a crash in embedded_test_server where HandleAcceptSocket() was called
without a socket, which would crash.

Bug:  844031 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I314d3fc7fb19cbbf486acce75799a6a3382cc1cd
Reviewed-on: https://chromium-review.googlesource.com/1064395
Reviewed-by: Misha Efimov <mef@chromium.org>
Commit-Queue: Paul Jensen <pauljensen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560572}
[modify] https://crrev.com/2771f9828d8b640dbdf79237c0002928627805ba/components/cronet/android/java/src/org/chromium/net/impl/JavaCronetEngine.java
[modify] https://crrev.com/2771f9828d8b640dbdf79237c0002928627805ba/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestTest.java
[modify] https://crrev.com/2771f9828d8b640dbdf79237c0002928627805ba/components/cronet/android/test/javatests/src/org/chromium/net/TestUrlRequestCallback.java
[modify] https://crrev.com/2771f9828d8b640dbdf79237c0002928627805ba/net/test/embedded_test_server/embedded_test_server.cc

Status: Fixed (was: Started)

Sign in to add a comment