Analysis of javac compiled classes can cause static initializers to run #151

Closed
stuhood opened this Issue Jul 13, 2016 · 5 comments

Comments

Projects
None yet
3 participants
@stuhood
Contributor

stuhood commented Jul 13, 2016

Changes previously introduced to use the JDK reflection APIs (rather than only the classfile parser) cause static initializers in loaded classes to run (which is never desirable, as it sometimes has sideeffects).

To avoid this, we need to avoid using any APIs that cause classes to load.

java.lang.ExceptionInInitializerError
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at java.lang.Class.getEnumConstantsShared(Class.java:3320)
    at java.lang.Class.getEnumConstants(Class.java:3297)
    at sbt.internal.inc.ClassToAPI$.childrenOfSealedClass(ClassToAPI.scala:306)
    at sbt.internal.inc.ClassToAPI$.toDefinitions0(ClassToAPI.scala:76)
    at sbt.internal.inc.ClassToAPI$$anonfun$toDefinitions$1.apply(ClassToAPI.scala:68)
    at sbt.internal.inc.ClassToAPI$$anonfun$toDefinitions$1.apply(ClassToAPI.scala:68)
    at scala.collection.mutable.MapLike$class.getOrElseUpdate(MapLike.scala:189)
    at scala.collection.mutable.AbstractMap.getOrElseUpdate(Map.scala:91)
    at sbt.internal.inc.ClassToAPI$.toDefinitions(ClassToAPI.scala:68)
    at sbt.internal.inc.ClassToAPI$$anonfun$4.apply(ClassToAPI.scala:23)
    at sbt.internal.inc.ClassToAPI$$anonfun$4.apply(ClassToAPI.scala:23)
    at scala.collection.TraversableLike$$anonfun$flatMap$1.apply(TraversableLike.scala:251)
    at scala.collection.TraversableLike$$anonfun$flatMap$1.apply(TraversableLike.scala:251)
    at scala.collection.mutable.ResizableArray$class.foreach(ResizableArray.scala:59)
    at scala.collection.mutable.ArrayBuffer.foreach(ArrayBuffer.scala:47)
    at scala.collection.TraversableLike$class.flatMap(TraversableLike.scala:251)
    at scala.collection.AbstractTraversable.flatMap(Traversable.scala:105)
    at sbt.internal.inc.ClassToAPI$.process(ClassToAPI.scala:23)
    at sbt.internal.inc.javac.AnalyzingJavaCompiler.sbt$internal$inc$javac$AnalyzingJavaCompiler$$readAPI$1(AnalyzingJavaCompiler.scala:81)
    at sbt.internal.inc.javac.AnalyzingJavaCompiler$$anonfun$compile$2$$anonfun$apply$mcV$sp$2$$anonfun$apply$5.apply(AnalyzingJavaCompiler.scala:91)
    at sbt.internal.inc.javac.AnalyzingJavaCompiler$$anonfun$compile$2$$anonfun$apply$mcV$sp$2$$anonfun$apply$5.apply(AnalyzingJavaCompiler.scala:91)
    at sbt.internal.inc.classfile.Analyze$$anonfun$apply$11.apply(Analyze.scala:65)
    at sbt.internal.inc.classfile.Analyze$$anonfun$apply$11.apply(Analyze.scala:60)
    at scala.collection.TraversableLike$WithFilter$$anonfun$foreach$1.apply(TraversableLike.scala:772)
    at scala.collection.mutable.HashMap$$anonfun$foreach$1.apply(HashMap.scala:98)
    at scala.collection.mutable.HashMap$$anonfun$foreach$1.apply(HashMap.scala:98)
    at scala.collection.mutable.HashTable$class.foreachEntry(HashTable.scala:226)
    at scala.collection.mutable.HashMap.foreachEntry(HashMap.scala:39)
    at scala.collection.mutable.HashMap.foreach(HashMap.scala:98)
    at scala.collection.TraversableLike$WithFilter.foreach(TraversableLike.scala:771)
    at sbt.internal.inc.classfile.Analyze$.apply(Analyze.scala:60)
    at sbt.internal.inc.javac.AnalyzingJavaCompiler$$anonfun$compile$2$$anonfun$apply$mcV$sp$2.apply(AnalyzingJavaCompiler.scala:91)
    at sbt.internal.inc.javac.AnalyzingJavaCompiler$$anonfun$compile$2$$anonfun$apply$mcV$sp$2.apply(AnalyzingJavaCompiler.scala:89)
    at scala.collection.TraversableLike$WithFilter$$anonfun$foreach$1.apply(TraversableLike.scala:772)
    at scala.collection.immutable.List.foreach(List.scala:318)
    at scala.collection.TraversableLike$WithFilter.foreach(TraversableLike.scala:771)
    at sbt.internal.inc.javac.AnalyzingJavaCompiler$$anonfun$compile$2.apply$mcV$sp(AnalyzingJavaCompiler.scala:89)
    at sbt.internal.inc.javac.AnalyzingJavaCompiler$$anonfun$compile$2.apply(AnalyzingJavaCompiler.scala:89)
    at sbt.internal.inc.javac.AnalyzingJavaCompiler$$anonfun$compile$2.apply(AnalyzingJavaCompiler.scala:89)
    at sbt.internal.inc.javac.AnalyzingJavaCompiler.timed(AnalyzingJavaCompiler.scala:100)
    at sbt.internal.inc.javac.AnalyzingJavaCompiler.compile(AnalyzingJavaCompiler.scala:88)
@stuhood

This comment has been minimized.

Show comment
Hide comment
@stuhood

stuhood Jul 13, 2016

Contributor

@eed3si9n : Something between com.typesafe.sbt;incremental-compiler#0.13.9 and org.scala-sbt;zinc#1.0.0-X1... honestly, I suspect it is fallout from sbt/sbt#2085, which we are just now attempting to incorporate.

Contributor

stuhood commented Jul 13, 2016

@eed3si9n : Something between com.typesafe.sbt;incremental-compiler#0.13.9 and org.scala-sbt;zinc#1.0.0-X1... honestly, I suspect it is fallout from sbt/sbt#2085, which we are just now attempting to incorporate.

@stuhood stuhood referenced this issue in pantsbuild/pants Jul 13, 2016

Open

Umbrella for upstream zinc issues #3669

@stuhood

This comment has been minimized.

Show comment
Hide comment
@stuhood

stuhood Jul 14, 2016

Contributor

Updated original with a stack trace.

Contributor

stuhood commented Jul 14, 2016

Updated original with a stack trace.

@romanowski

This comment has been minimized.

Show comment
Hide comment
@romanowski

romanowski Aug 26, 2016

Contributor

@stuhood can you confirm that this is still a problem after #158 is merged?

I faced similar problem and remove part of ClassToAPI that create new instances of enums classes in zinc runtime.

Contributor

romanowski commented Aug 26, 2016

@stuhood can you confirm that this is still a problem after #158 is merged?

I faced similar problem and remove part of ClassToAPI that create new instances of enums classes in zinc runtime.

@stuhood

This comment has been minimized.

Show comment
Hide comment
@stuhood

stuhood Oct 18, 2016

Contributor

Thanks @romanowski : @peiyuwang confirmed that this was fixed for us.

Contributor

stuhood commented Oct 18, 2016

Thanks @romanowski : @peiyuwang confirmed that this was fixed for us.

@stuhood stuhood closed this Oct 18, 2016

peiyuwang added a commit to pantsbuild/pants that referenced this issue Nov 9, 2016

Upgrade zinc's sbt dependency to 1.0.0: JVM portion
This review is based off Stu's unsubmitted previous effort: https://rbcommons.com/s/twitter/r/3658/

The python portion is in https://rbcommons.com/s/twitter/r/4342/

It has everything from rb/3658:

* Update for new sbt/zinc repo, that also includes a few fixes for us:
  ** empty analysis sbt/zinc#144
  ** class file analysis trigger static initializers run sbt/zinc#151
* Exclude the existing io/logging deps, and re-include them explicitly as forced (with the appropriate classifiers)
* Update all imports/dependencies to new locations
* Require an explicit -cache-dir (which will be placed inside the pants cache directory in the review that incorporates this * version)
* Remove the -name-hashing flag, as name hashing is required for the now-default class-based dependency tracking.
* Rename sbt-interface to compiler-interface, and compiler-interface to compiler-bridge. Confused yet?

Plus a few other changes:

* Analysis format recently changed to a zip with two entries. This review keeps the plain txt format pants parser uses. Long term we probably should switch to some internal format that's more stable and lighter weight.
* An option to turn off zinc provided file manager, see sbt/zinc#185
* re-enable the optimization to check class existence from analysis (Significant performance impact, esp. for incremental compile)

Testing Done:
https://travis-ci.org/peiyuwang/pants/builds/172980195
https://travis-ci.org/peiyuwang/pants/builds/174415697

Bugs closed: 3962, 4021

Reviewed at https://rbcommons.com/s/twitter/r/4340/
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment