facebook / hermes

A JavaScript engine optimized for running React Native.
https://hermesengine.dev/
MIT License
9.91k stars 641 forks source link

fetch() BlobModule.java memory leak still exists on hermes-engine 0.9.0 #653

Closed dov11 closed 4 weeks ago

dov11 commented 2 years ago

Bug Description

The issue described in #164 still exists in React Native 0.66.3 that comes with hermes-engine 0.9.0. I have initiated a fresh RN project and used the same code as in this comment. I experience the same crash if hermes engine is enabled. I do not see it with JSC.

Hermes version: 0.9.0 React Native version (if any): 0.66.3 OS version (if any): Platform (most likely one of arm64-v8a, armeabi-v7a, x86, x86_64):

Steps To Reproduce

  1. Clone https://github.com/dov11/hermesleak
  2. Build and run the app
  3. Experience crash at around 190MB downloaded
  4. Switch to hermes_disabled branch
  5. Run gradle clean
  6. Build and run the app
  7. No crash

Crash happens on both debug and release builds. Tested on Galaxy S7 device.

The Expected Behavior

No memory leak crash with hermes engine enabled

neildhar commented 2 years ago

Hi @dov11, thanks for reporting this and for the detailed repro steps. There were two causes discussed in that original issue, one of them was a bug in Hermes, and the other is a resource management problem for applications. The bug in Hermes is fixed, however, it is still possible to trigger a Java OOM, depending on how you manage resources.

Each blob is very small in the JS heap, but retains a large amount of memory in the Java heap. Since the Hermes GC is not aware of the resources being retained in Java, it doesn't run frequently enough to make sure that those resources are freed and prevent a Java OOM. So while the original repro still triggers a crash, if you add a call to gc() (to force a garbage collection cycle), it doesn't crash any more.

There are multiple ways to fix this. The best thing to do is to manually release the resource once you're done with it instead of waiting for the GC to clean it up. As a backstop, you could manually trigger a GC whenever Java is experiencing memory pressure.

jg210 commented 2 years ago

From react-native v0.63.0-rc.,0 js garbage collections should be triggered if android indicates that memory pressure is high:

https://github.com/facebook/react-native/issues/27532 https://github.com/facebook/react-native/commit/48001c597eba1c9f4eafb6b47e0b9f758e6c424f

Why is that not working/helping here for hermes?

neildhar commented 2 years ago

From reading the Android docs, I suspect that this monitors the process' total memory consumption, but perhaps not whether the Java heap is approaching its configured maximum size. In testing, I found that the Java OOM occurred at a size of 512MB, which was perhaps not enough to trigger a memory warning for the app and force a GC cycle.

tmikov commented 4 weeks ago

Closing, since no activity and not actionable.