Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cronet with Retrofit is crashing due to file-descriptor leaks #15

Open
marius-bardan opened this issue Jan 19, 2023 · 4 comments
Open

Comments

@marius-bardan
Copy link

CronetCallFactory.newBuilder(engine).build()
val builder = Retrofit.Builder()
builder.callFactory(factory)
// use builder with OkHttp

Before enabling Cronet with Retrofit using above snippet, make sure to chek the file-descriptor count, via:

  1. [standard emulator] adb shell
  2. su
  3. ls -l /proc/$(pidof APP_PACKAGE_NAME)/fd | wc [we're interested in the first value]

Running the same commands again, after we enable Cronet with Retrofit almost doubled the number of opened file descriptors for my app, which on some devices is causing a crash, since it hits the limit per one app.

@edechamps-Google
Copy link
Collaborator

Thanks for the feedback.

Using the sample app, and the following command on an ADB root shell:

for fd in /proc/$(pidof com.google.samples.cronet.okhttptransport)/fd/*; do readlink "$fd"; done | sort

FD counts:

  • Sample app start: 84
  • After sending requests using pure OkHttp: 88
  • After initializing Cronet: 104
  • After sending requests using Cronet: 106

4 additional FDs when sending requests using pure OkHttp:

+/dev/ashmem
+pipe:[93397]
+pipe:[93397]
+socket:[93392]

20 additional FDs when initializing Cronet:

+/data/app/com.google.android.gms-44whme714CrlclOYAuJT3w==/base.apk
+/data/user_de/0/com.google.android.gms/app_chimera/m/00000001/CronetDynamite.apk
+/data/user_de/0/com.google.android.gms/app_chimera/m/00000001/CronetDynamite.apk
+/dev/ashmem
+/dev/urandom
+anon_inode:[eventfd]
+anon_inode:[eventfd]
+anon_inode:[eventpoll]
+anon_inode:[eventpoll]
+anon_inode:[eventpoll]
+anon_inode:[timerfd]
+anon_inode:[timerfd]
+pipe:[95093]
+pipe:[95093]
+pipe:[95353]
+pipe:[95353]
+socket:[95091]
+socket:[95092]
+socket:[95351]
+socket:[95352]

2 additional FDs when sending requests through Cronet:

+socket:[96661]
+socket:[98600]

So using Cronet results in 18 additional FDs being used, compared to pure OkHttp.

I also looked at the number of FDs being opened per contacted destination domain, but I couldn't find a clear regression between pure OkHttp and Cronet-backed OkHttp in that regard - they both seem to use 1 to 2 FD per destination domain.

To clarify, is it the 20 FDs fixed cost from Cronet initialization that's bothering you or are you observing behavior that isn't covered in the above?

@marius-bardan
Copy link
Author

I think it depends on the number of requests per app.
In case of an enterpise app with tens of requests being executed, we're hitting the process limit of opened FDs and OS kills the app.
We're not doing anything extra here - just using OkHttp vanilla vs using it with the Cronet interceptor (as basic as it can get).

@Danstahrg
Copy link
Contributor

Danstahrg commented May 12, 2023

I've tried issuing 1k concurrent OkHttp requests to the top 1k websites as indicated by this list: https://gist.github.com/bejaneps/ba8d8eed85b0c289a05c750b3d825f61. Like Etienne, I haven't experienced any issues with the setup and the number of open descriptors looks roughly consistent as the requests re being processed, with the exception of Cronet being a bit more aggressive with keeping them open towards the end. This was using the default Pixel 6 emulator with Android 13.

If you can reliably reproduce the issue in a sandbox setup repro steps would be much appreciated.

@marius-bardan
Copy link
Author

@Danstahrg can you please share the sample app code for that 1k requests test?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants