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

Crashes with "apply changes" (SIGABRT) #134

Open
PaulWoitaschek opened this issue Jul 4, 2019 · 10 comments
Open

Crashes with "apply changes" (SIGABRT) #134

PaulWoitaschek opened this issue Jul 4, 2019 · 10 comments

Comments

@PaulWoitaschek
Copy link
Contributor

When using Android Studios "Apply changes" (3.5.0-beta05) with dagger reflect, upon reload it crashes on the next injection.

https://gist.github.com/PaulWoitaschek/53c918e4947c4ade5c04642dedbacf45

@JakeWharton
Copy link
Owner

Seems like you should file this against the tools and see what they say first? I'm not sure there's a whole lot we can do here except for caching nothing which will destroy what little performance choices we've made. My best guess is you changed the shape of a class where we already cached its information via reflection. Using these reflective objects after the change causes ART to crash with a nasty error which, if they want to keep crashing, should probably have something a bit more user-friendly.

@PaulWoitaschek
Copy link
Contributor Author

@acleung
Copy link

acleung commented Jan 8, 2020

@JakeWharton: What's getting cached by Dagger? j/l/Class, j/l/reflect/Field,Method ?

@JakeWharton
Copy link
Owner

Probably all three. Keys in the core map contain references to Classes and the associated map values contain references to Methods or Fields. Unfortunately it's somewhat fundamental to how the library works.

@allight
Copy link

allight commented Jan 9, 2020

This is caused by an oversight in the class-redefinition functionality underlying "apply changes" leading j.l.r.Field objects to become invalid. There's not really anything one would be able to do at this level to work around or avoid this bug unfortunately.

Until it's fixed in the platform or worked-around in apply-changes I'm not sure there's anything to do except avoid apply-changes on code using this.

@acleung
Copy link

acleung commented Jan 9, 2020

@JakeWharton: Is there a way to turn off caching of reflection objects for development builds? Given that Apply Changes only works on Debug builds and developers are probably more interested in rapid compile and deploy cycles than in-app performance at this point. Turning that off in Debug builds can be a workaround for those who are using older version of Android.

@JakeWharton
Copy link
Owner

The entry point to the library is a user supplying instances of a Class so to some degree it's impossible for the core functionality to work without retaining some of these references. If it's only Fields that are the problem I can potentially look at avoiding retaining their references.

@allight
Copy link

allight commented Jan 9, 2020

@JakeWharton Only j.l.r.Field objects can end up invalid. Not retaining any will make the problem less likely to occur. There should be no problems with caching Class or Method objects.

@JakeWharton
Copy link
Owner

👍 thanks for clarifying!

@PaulWoitaschek
Copy link
Contributor Author

The last comment in the upstream issue is:

It is a bug in the Android Runtime and it has been fixed in master.

Until the next release, one suggestion is to have dagger not to cache any reflect Field objects around for debug builds. That discussion has started in the dagger bug entry in #1

Will that workaround be implemented in dagger reflect?

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

4 participants