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

OutOfMemoryError comes back when targeting android 31 #6872

Closed
calvarez-ov opened this issue Nov 19, 2021 · 14 comments · Fixed by #6873
Closed

OutOfMemoryError comes back when targeting android 31 #6872

calvarez-ov opened this issue Nov 19, 2021 · 14 comments · Fixed by #6873

Comments

@calvarez-ov
Copy link

calvarez-ov commented Nov 19, 2021

Description

We had issue #2584.

We had this workaround for that issue, on every test tearDown:

        // https://github.com/robolectric/robolectric/issues/2584
        val windowData = Reflection.staticField("WINDOW_DATA").ofType(Any::class.java)
            .`in`(ShadowCursorWindow::class.java).get()
        val dataMap =
            Reflection.field("dataMap").ofType(MutableMap::class.java).`in`(windowData).get()
        dataMap.clear()

With robolectric 4.7.1, if we set sdk=31 in robolectric.properties, this workaround causes reflection exceptions when running tests.

I removed the workaround, and I have OutOfMemoryErrors once again.

Steps to Reproduce

See #2584

Robolectric & Android Version

  • robolectric: 4.7.1
  • compileSdkVersion: 31
  • targetSdkVersion: 30
  • robolectric.properties: sdk: 31

Note: with robolectric 4.7.1, if stay with sdk of 30 and keep the workaround, there aren't OutOfMemoryErrors
Not so sure about this statement. I need to double check.

@calvarez-ov
Copy link
Author

If I make the following change the workaround, I still have OutOfMemoryErrors:

         // https://github.com/robolectric/robolectric/issues/2584
         val windowData = Reflection.staticField("WINDOW_DATA").ofType(Any::class.java)
-            .`in`(ShadowCursorWindow::class.java).get()
+            .`in`(ShadowLegacyCursorWindow::class.java).get()
         val dataMap =
             Reflection.field("dataMap").ofType(MutableMap::class.java).`in`(windowData).get()
         dataMap.clear()

@hoisie
Copy link
Contributor

hoisie commented Nov 19, 2021

Can you add -XX:+HeapDumpOnOutOfMemoryError to your project's JVM flags? It would be good to track what is causing the OOM. You could either upload the hprof file or use VisualVM to analyze it.

Also what's the max heap size that is being used? I typically recommend -Xmx4096M.

@calvarez-ov
Copy link
Author

I'll check those jvm flags in a moment.

I just wanted to also note here that I tried setting the following, and it didn't solve the problem:

     testOptions {
         unitTests {
             all {
                 ...
                 systemProperty 'robolectric.sqliteMode', "NATIVE"
             }
         }
     }

@calvarez-ov
Copy link
Author

calvarez-ov commented Nov 19, 2021

I had almost your settings, but in a comment:

#org.gradle.jvmargs=-Xmx4G -XX:+HeapDumpOnOutOfMemoryError

Our deveops person recommended to not specify them:

My recommendation would be to remove all the specific configurations (Xmx as you did) and to rely only on the memory of the container

But I'll uncomment it and try again

@calvarez-ov
Copy link
Author

Ok, I found the config to enable these options:

 tasks.configureEach { theTask ->
     def taskName = theTask.name.toString()
     if (taskName =~ /test.*DebugUnitTest/) {
-        theTask.jvmArgs('-ea', '-noverify')
+        theTask.jvmArgs('-ea', '-noverify', '-Xmx4G', '-XX:+HeapDumpOnOutOfMemoryError')
     }
 }

I now have a 2G heap file! I'll check out VisualVM

@calvarez-ov
Copy link
Author

image

@calvarez-ov
Copy link
Author

image

@hoisie
Copy link
Contributor

hoisie commented Nov 19, 2021

Can you search for Activity? When you see resources like that, it typically involves Activities that leak across tests.

@calvarez-ov
Copy link
Author

For info, doing this at the end of each test makes the OutOfMemoryErrors go away:

Registries.NATIVE_THEME9_REGISTRY.clear()

@calvarez-ov
Copy link
Author

I'll look for activites though

@calvarez-ov
Copy link
Author

Possibly related: #5530

@hoisie
Copy link
Contributor

hoisie commented Nov 19, 2021

Thanks for reporting this. Looking at this further, it does seem like the logic that frees themes does not seem to run in Android S:

https://github.com/robolectric/robolectric/blob/master/shadows/framework/src/main/java/org/robolectric/shadows/ShadowArscAssetManager10.java#L1585

This seems to be the logic for releasing themes now:

https://cs.android.com/android/platform/superproject/+/master:frameworks/base/core/java/android/content/res/AssetManager.java;l=1180-1184

This ends up calling nativeDestroy.

copybara-service bot pushed a commit that referenced this issue Nov 20, 2021
Previously, in Android P through R, Robolectric would clean up native themes
from the registry when AssetManager.nativeThemeDestroy was called by the
ThemeImpl finalizer. However, starting in Android S,
AssetManager.nativeThemeDestroy does not exist any more.

As a workaround, hook into the AssetManager.releaseTheme method to free the
theme from the registry. Longer term we may want to have the theme destruction
logic be part of AssetManager.nativeDestroy, where all themes can potentially
be freed in one fell swoop.

This should fix #6872. Thanks to @calvarez-ov for reporting the issue and
helping debug :)

PiperOrigin-RevId: 411169531
hoisie added a commit that referenced this issue Nov 20, 2021
Previously, in Android P through R, Robolectric would clean up native themes
from the registry when AssetManager.nativeThemeDestroy was called by the
ThemeImpl finalizer. However, starting in Android S,
AssetManager.nativeThemeDestroy does not exist any more.

As a workaround, hook into the AssetManager.releaseTheme method to free the
theme from the registry. Longer term we may want to have the theme destruction
logic be part of AssetManager.nativeDestroy, where all themes can potentially
be freed in one fell swoop.

This should fix #6872. Thanks to @calvarez-ov for reporting the issue and
helping debug :)

PiperOrigin-RevId: 411169531
hoisie added a commit that referenced this issue Nov 20, 2021
Previously, in Android P through R, Robolectric would clean up native themes
from the registry when AssetManager.nativeThemeDestroy was called by the
ThemeImpl finalizer. However, starting in Android S,
AssetManager.nativeThemeDestroy does not exist any more.

As a workaround, hook into the AssetManager.releaseTheme method to free the
theme from the registry. Longer term we may want to have the theme destruction
logic be part of AssetManager.nativeDestroy, where all themes can potentially
be freed in one fell swoop.

This should fix #6872. Thanks to @calvarez-ov for reporting the issue and
helping debug :)

PiperOrigin-RevId: 411169531
hoisie added a commit that referenced this issue Nov 20, 2021
Previously, in Android P through R, Robolectric would clean up native themes
from the registry when AssetManager.nativeThemeDestroy was called by the
ThemeImpl finalizer. However, starting in Android S,
AssetManager.nativeThemeDestroy does not exist any more.

As a workaround, hook into the AssetManager.releaseTheme method to free the
theme from the registry. Longer term we may want to have the theme destruction
logic be part of AssetManager.nativeDestroy, where all themes can potentially
be freed in one fell swoop.

This should fix #6872. Thanks to @calvarez-ov for reporting the issue and
helping debug :)

PiperOrigin-RevId: 411169531
hoisie added a commit that referenced this issue Nov 20, 2021
Previously, in Android P through R, Robolectric would clean up native themes
from the registry when AssetManager.nativeThemeDestroy was called by the
ThemeImpl finalizer. However, starting in Android S,
AssetManager.nativeThemeDestroy does not exist any more.

As a workaround, hook into the AssetManager.releaseTheme method to free the
theme from the registry. Longer term we may want to have the theme destruction
logic be part of AssetManager.nativeDestroy, where all themes can potentially
be freed in one fell swoop.

This should fix #6872. Thanks to @calvarez-ov for reporting the issue and
helping debug :)

PiperOrigin-RevId: 411169531
@hoisie
Copy link
Contributor

hoisie commented Nov 20, 2021

Just pushed 4.7.2 which should hopefully contain a fix for this issue. Reopening for now. @calvarez-ov when you get a chance can you check if it worked?

@hoisie hoisie reopened this Nov 20, 2021
@caarmen
Copy link

caarmen commented Nov 21, 2021

Yes, it works!

I updated to 4.7.2, removed my Registries.NATIVE_THEME9_REGISTRY.clear() workaround, and ran the test suite without any OutOfMemoryError.

Thanks so much! 🙏

(I'm calvarez-ov's evil twin)

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

Successfully merging a pull request may close this issue.

3 participants