dagger-reflect icon indicating copy to clipboard operation
dagger-reflect copied to clipboard

Crashes with "apply changes" (SIGABRT)

Open PaulWoitaschek opened this issue 5 years ago • 10 comments

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

PaulWoitaschek avatar Jul 04 '19 12:07 PaulWoitaschek

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.

JakeWharton avatar Jul 05 '19 21:07 JakeWharton

Filed https://issuetracker.google.com/issues/147310999

PaulWoitaschek avatar Jan 08 '20 08:01 PaulWoitaschek

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

acleung avatar Jan 08 '20 09:01 acleung

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.

JakeWharton avatar Jan 08 '20 17:01 JakeWharton

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.

allight avatar Jan 09 '20 00:01 allight

@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.

acleung avatar Jan 09 '20 02:01 acleung

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.

JakeWharton avatar Jan 09 '20 21:01 JakeWharton

@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.

allight avatar Jan 09 '20 23:01 allight

👍 thanks for clarifying!

JakeWharton avatar Jan 11 '20 04:01 JakeWharton

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?

PaulWoitaschek avatar Jan 25 '20 13:01 PaulWoitaschek