Don't bind synthetic classes to Ruby#8843
Conversation
Synthetic classes, such as our own "InterfaceImpl" proxies generated on-the-fly to adapt Ruby objects to Java interfaces, are generally not supposed to be visible to users. A change in JRuby 9.4 to reduce wrapping overhead for InterfaceImpl proxy objects led to them also being bound as Ruby proxy classes, which had the effect of causing these frequently-generated and frequently- abandoned classes to be pinned in memory by the proxy class's method bindings. This led to a class and metaspace leak reported in jruby#8842. The fix here explicitly avoids binding any synthetic classes, instead skipping such classes and walking up to the nearest non- synthetic class before beginning the binding process. This avoids our InterfaceImpl classes from being bound as Ruby proxies and appears to eliminiate at least the leak reproduced in jruby#8842. This may also fix other undiscovered or un-reproduced cases of generated classes leaking into the Ruby proxy class hierarchy, so long as all of those classes are being generated as synthetic. This may also be a more direct and appropriate fix for the eager binding and logging of InterfaceImpl classes reported in jruby#8349, which was previously fixed by avoiding synthetic classes when adding proxies to our Java package namespace structures (jruby#8503). If that is the case, we may want to consider turning that check into an error, since it will probably always be incorrect to have synthetic classes get bound into a public namespace. Additional checks elsewhere in our Java Integration subsystem could also help prevent similar issues in the future, and we should audit all generated classes to ensure they are marked as synthetic. Fixes jruby#8842. May be a better fix for jruby#8349 than jruby#8503 but that fix's check should perhaps remain or become a hard error. Needs tests for as many JI class generation cases as possible.
|
Note also that this would not have been as serious an issue if we were reusing InterfaceImpl classes that represented the same superclass and interfaces. Unfortunately, as reported in other issues like #5023, we do no caching of such transient implementation proxies, and constantly regenerate them for each new object to be adapted. We should also fix that issue, but due to the more invasive nature of such a fix it might be safest to do it only on the active-development JRuby 10 line. |
|
The tests indicate that synthetic is too broad a net to cast. Lambda expressions result in interface implementation classes that are synthetic, but they do not have a superclass with the requisite methods to be invoked. We probably need to mark our generated classes with an additional interface and use that to indicate that they should not be bound as proxy classes. This issue could also affect the fix in #8503 but at least lambda expressions don't have a named class we can use directly and would not go into the package structure. |
|
My experiments trying to ignore interface impl classes led to a few discoveries:
See #8844. |
|
Wrong approach; we do actually need to bind these classes so there's a place for their interface methods to live, and lambda expressions are synthetic and invoking them from Ruby breaks with this change. See #8844 for a better fix. |
2 participants
Synthetic classes, such as our own "InterfaceImpl" proxies generated on-the-fly to adapt Ruby objects to Java interfaces, are generally not supposed to be visible to users. A change in JRuby 9.4 to reduce wrapping overhead for InterfaceImpl proxy objects led to them also being bound as Ruby proxy classes, which had the effect of causing these frequently-generated and frequently- abandoned classes to be pinned in memory by the proxy class's method bindings. This led to a class and metaspace leak reported in #8842.
The fix here explicitly avoids binding any synthetic classes, instead skipping such classes and walking up to the nearest non- synthetic class before beginning the binding process. This avoids our InterfaceImpl classes from being bound as Ruby proxies and appears to eliminiate at least the leak reproduced in #8842.
This may also fix other undiscovered or un-reproduced cases of generated classes leaking into the Ruby proxy class hierarchy, so long as all of those classes are being generated as synthetic.
This may also be a more direct and appropriate fix for the eager binding and logging of InterfaceImpl classes reported in #8349, which was previously fixed by avoiding synthetic classes when adding proxies to our Java package namespace structures (#8503). If that is the case, we may want to consider turning that check into an error, since it will probably always be incorrect to have synthetic classes get bound into a public namespace. Additional checks elsewhere in our Java Integration subsystem could also help prevent similar issues in the future, and we should audit all generated classes to ensure they are marked as synthetic.
Fixes #8842.
May be a better fix for #8349 than #8503 but that fix's check should perhaps remain or become a hard error.
Needs tests for as many JI class generation cases as possible.