codingwell / scala-guice

Scala extensions for Google Guice
Apache License 2.0
341 stars 44 forks source link

Problems with Play framework's dev mode after upgrading to scala-guice 4.2.2 #77

Closed skress closed 5 years ago

skress commented 5 years ago

First of all: thanks a lot for maintaining this library, it is really helpful.

I've run into problems with scala-guice after upgrading to 4.2.2 in combination with Play framework's dev mode. There is a minimal Play project which you could use to reproduce the error: https://github.com/skress/issue-play-scala-guice

If I am not doing anything terribly wrong then even this simple configuration does not work:

trait Service {
  def message: String
}

class ServiceProvider extends Service {
  def message: String = "..."
}

class Module extends ScalaModule {

  override def configure() {
    bind[Service].to[ServiceProvider]
  }

}

The exception looks like this:

java.lang.ClassNotFoundException: modules.Service
     java.net.URLClassLoader.findClass(URLClassLoader.java:382)
     java.lang.ClassLoader.loadClass(ClassLoader.java:424)
     java.lang.ClassLoader.loadClass(ClassLoader.java:357)
     java.lang.Class.forName0(Native Method)
     java.lang.Class.forName(Class.java:348)
     scala.reflect.runtime.JavaMirrors$JavaMirror.javaClass(JavaMirrors.scala:570)
     scala.reflect.runtime.JavaMirrors$JavaMirror.$anonfun$classToJava$1(JavaMirrors.scala:1246)
     scala.reflect.runtime.TwoWayCaches$TwoWayCache.$anonfun$toJava$1(TwoWayCaches.scala:61)
     scala.reflect.runtime.TwoWayCaches$TwoWayCache.toJava(TwoWayCaches.scala:57)
     scala.reflect.runtime.JavaMirrors$JavaMirror.classToJava(JavaMirrors.scala:1238)
     scala.reflect.runtime.JavaMirrors$JavaMirror.runtimeClass(JavaMirrors.scala:209)
     scala.reflect.runtime.JavaMirrors$JavaMirror.runtimeClass(JavaMirrors.scala:67)
     net.codingwell.scalaguice.TypeConversions$.scalaTypeToJavaType(TypeConversions.scala:66)
     net.codingwell.scalaguice.package$.typeLiteral(package.scala:38)
     net.codingwell.scalaguice.InternalModule$BindingBuilder.<init>(ScalaModule.scala:69)
     net.codingwell.scalaguice.InternalModule.bind(ScalaModule.scala:74)
     net.codingwell.scalaguice.InternalModule.bind$(ScalaModule.scala:74)
     modules.Module.bind(Module.scala:13)

Reverting the two changes from Manifest to TypeTag in ScalaModule.scala (https://github.com/codingwell/scala-guice/commit/8ebf64e57d08e08c9472210a5bac694d5a99e2ba#diff-3f5a5d294b9751c91b703f2e10f25a04) seems to fix the issue.

The reason for the problem seems to be that the classloader that scala-guice sees is not the classloader that Play uses to load the project's classes in dev mode (running the app in production mode does not exhibit the error.) In Play's dev mode the project's dependencies are loaded, the framework is started and Play then creates a new classloader that loads the project's classes. When you change anything in your project the changes are compiled, the classloader is abandoned and a new one is created that loads the just compiled classes.

From my limited understanding this should explain why scala-guice does not work. But it should have never worked. What's the difference between Manifest and TypeTag that one is working and the other is not, even if in both cases the same (wrong) classloader is used?

tsuckow commented 5 years ago

This is almost certainly https://github.com/playframework/playframework/issues/6207

Which leads us to TypeConversions.scala#L66 which uses mirror which is indeed bound to the classloader of this library and not the classloader of the type in question.

I wonder though if it could simply be swapped for scalaType.dealias.getClass, or if there is some reason @Vrolijkx didn't do that in the first place.

skress commented 5 years ago

If you meant getting the mirror like runtimeMirror(scalaType.dealias.getClass.getClassLoader), then this did not work.

What helped is adding the mirror as parameter to TypeConversions.scala#L60 and getting the mirror from the TypeTag.

I have implemented it in a fork and it seems to work (i.e. my play project works now): https://github.com/skress/scala-guice/commit/913264e67ee91340eca9af4e1e55a67a19970074

I can create a PR - but I am not sure if there are any other (unwanted) side effects of that change.

tsuckow commented 5 years ago

Assuming you ran the tests and they passed, that should be fine. Make a PR and I'll test locally too.