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

"final var", public fields (!) and the inliner #3569

Closed
scabug opened this issue Jun 15, 2010 · 7 comments
Closed

"final var", public fields (!) and the inliner #3569

scabug opened this issue Jun 15, 2010 · 7 comments
Assignees
Labels

Comments

@scabug
Copy link

scabug commented Jun 15, 2010

Discovered looking at inliner tickets.

object Bop {
  class X(final var x: Int)  { }
  def f = new X(0).x += 1
  def main(args: Array[String]) {
    f
  }
}

Now if you turn on -Yinliner:

public final int x;

and the result:

java.lang.IllegalAccessError
	at Bop$$.main(finalvars.scala:6)
	at Bop.main(finalvars.scala)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
	at java.lang.reflect.Method.invoke(Method.java:597)
	at scala.tools.nsc.util.ScalaClassLoader$$$$anonfun$$run$$1.apply(ScalaClassLoader.scala:81)
	at scala.tools.nsc.util.ScalaClassLoader$$class.asContext(ScalaClassLoader.scala:24)
	at scala.tools.nsc.util.ScalaClassLoader$$URLClassLoader.asContext(ScalaClassLoader.scala:86)
	at scala.tools.nsc.util.ScalaClassLoader$$class.run(ScalaClassLoader.scala:81)
	at scala.tools.nsc.util.ScalaClassLoader$$URLClassLoader.run(ScalaClassLoader.scala:86)
	at scala.tools.nsc.ObjectRunner$$.run(ObjectRunner.scala:31)
	at scala.tools.nsc.MainGenericRunner$$.main(MainGenericRunner.scala:83)
	at scala.tools.nsc.MainGenericRunner.main(MainGenericRunner.scala)
@scabug
Copy link
Author

scabug commented Jun 15, 2010

Imported From: https://issues.scala-lang.org/browse/SI-3569?orig=1
Reporter: @paulp

@scabug
Copy link
Author

scabug commented Jun 15, 2010

@dragos said:
The Scala spec allows final on vars, since it only restricts the ability to override. It is indeed a bit fishy that it ends up as final in the bytecode. Final is triply overloaded on the JVM: overriding, immutability and synchronization, all in one. So we need a way to signal fields as being final, for they have special initialization semantics in a multithreaded environment.

Mutation on final fields is allowed (at the JVM level) only from the defining class of the field. This is definitely a bug on the part of the inliner, anyway. If you run the code it throws an IllegalAccessError.

Paul, since you are 'cozy' with the inliner, would you like to tweak the safety check to disallow this?

@scabug
Copy link
Author

scabug commented Dec 30, 2010

@paulp said:
This is worse than something obscure like final var. The following class generates a public field for i if compiled with -Yinline.

final class D(val i: Int) { 
  def f = i*2
}
// public final int i;

On the other hand the example in the ticket doesn't generate a runtime error anymore, because apparently nothing is inlined. Which is either another bug (I am having a lot of trouble matching up what I imagine the inliner to do with what I see it doing) or some penetrating insight on its part.

@scabug
Copy link
Author

scabug commented Jun 14, 2011

@paulp said:
See also #4643.

@scabug
Copy link
Author

scabug commented Mar 7, 2012

@adriaanm said (edited on Mar 7, 2012 2:45:35 PM UTC):
from https://groups.google.com/d/topic/scala-user/bjar0Qcl9eA/discussion

this produces bytecode that assigns repeatedly to vars marked final,
which results in unpredictable run-time behavior (well, it's predictably wrong)

final class Box {
  private[this] final var TMP:Object = null

  final def put(o:Object) {
    TMP = o
  }

  final def get() = {
    val tmp = TMP
    TMP = null
    tmp
  }
}

class Client(box:Box) {
  var count = 0

  final def run(cycles:Int) {
    if (cycles > 0) {
      box.put(new Object)
      if (box.get() != null) count += 1
      run(cycles - 1)
    }
  }
}

object Main {

  def main(args:Array[String]) {
    val c = new Client(new Box)
    val N = 1000000000
    c.run(N)
    assert(c.count == N, c.count)
  }

}

$ sbt run
[error] (run-main) java.lang.AssertionError: assertion failed: 999982178
java.lang.AssertionError: assertion failed: 999982178
        at scala.Predef$.assert(Predef.scala:103)
        at Main$.main(Bug.scala:33)
        at Main.main(Bug.scala)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
        at java.lang.reflect.Method.invoke(Method.java:597)
java.lang.RuntimeException: Nonzero exit code: 1
        at scala.sys.package$.error(package.scala:27)

$ java -version
java version "1.6.0_26"
Java(TM) SE Runtime Environment (build 1.6.0_26-b03)
Java HotSpot(TM) Server VM (build 20.1-b02, mixed mode)

$ scala -version
Scala code runner version 2.9.1.final -- Copyright 2002-2011, LAMP/EPFL

@scabug
Copy link
Author

scabug commented Mar 7, 2012

@odersky said:
I think this is a blocker for 2.10 and also for 2.9.2

@scabug
Copy link
Author

scabug commented Apr 8, 2012

@paulp said:
Should be fixed in 399bd6240f for trunk and also in 2.9.2.

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

No branches or pull requests

2 participants