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

java.lang.IllegalAccessError in lzycompute in Java 11 #11781

Closed
leitoh0 opened this issue Oct 27, 2019 · 5 comments
Closed

java.lang.IllegalAccessError in lzycompute in Java 11 #11781

leitoh0 opened this issue Oct 27, 2019 · 5 comments

Comments

@leitoh0
Copy link

leitoh0 commented Oct 27, 2019

Newer JVMs (11 at least) throw a java.lang.IllegalAccessError on detecting an update to non-static final field <foo> attempted from a different method (<bar>$lzycompute) than the initializer method <init>.

fail$lzcompute() in the following code would contain such an illegal putfield.

class C
class Fail(c: C) {
  lazy val fail = c
}

Perhaps the best solution is to de-(java-)finalize single-use private immutable fields, at least the ones slated for nullification?

MixinTransformer.lazyValNullables could exclude (java-)final fields, exposing potential GC cost and turning itself mostly pointless in the process, so that may be a less-desirable option.

It is possible that there are other places where such now-illegal field nullification could be taking place (aside from lzycompute). IMHO backend does not seem to do so for fields, though.

@leitoh0
Copy link
Author

leitoh0 commented Oct 28, 2019

E.g. something of this nature:

--- a/src/compiler/scala/tools/nsc/transform/Mixin.scala
+++ b/src/compiler/scala/tools/nsc/transform/Mixin.scala
@@ -453,8 +453,10 @@ abstract class Mixin extends Transform with ast.TreeDSL with AccessorSynthesis {
             deriveDefDef(dd) {
               case blk@Block(stats, expr) =>
                 assert(dd.symbol.originalOwner.isClass, dd.symbol)
-                def nullify(sym: Symbol) =
+                def nullify(sym: Symbol) = {
+                  sym.accessedOrSelf.setFlag(MUTABLE)
                   Select(gen.mkAttributedThis(sym.enclClass), sym.accessedOrSelf) === NULL
+                }
                 val stats1 = stats ::: fieldsToNull.map(nullify)
                 treeCopy.Block(blk, stats1, expr)
               case tree =>

@hrhino hrhino added the jdk11 label Oct 29, 2019
@hrhino
Copy link

hrhino commented Oct 29, 2019

Hi @leitoh0:

That looks reasonable to me. Would you mind submitting it as a patch?

@leitoh0
Copy link
Author

leitoh0 commented Oct 29, 2019

Hi @hrhino,

Thanks for checking. Not at all: scala/scala#8514
Do you want a 2.13.x PR separately (same diff), or is the above sufficient?

@hrhino
Copy link

hrhino commented Oct 29, 2019

Thanks! 2.12.x only is fine; it gets merged into 2.13.x periodically.

@som-snytt
Copy link

Requires -target:9 and up, if anyone tries it and wonders.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants