Skip to content

Fixes to lambdalift that prevent memory leaks. #1281

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

Merged
merged 4 commits into from
May 27, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/dotty/tools/dotc/transform/ElimStaticThis.scala
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,11 @@ class ElimStaticThis extends MiniPhaseTransform {
override def transformIdent(tree: tpd.Ident)(implicit ctx: Context, info: TransformerInfo): tpd.Tree = {
if (ctx.owner.enclosingMethod.is(JavaStatic)) {
tree.tpe match {
case TermRef(thiz: ThisType, _) =>
assert(thiz.underlying.typeSymbol.is(ModuleClass))
case TermRef(thiz: ThisType, _) if thiz.underlying.typeSymbol.is(ModuleClass) =>
ref(thiz.underlying.typeSymbol.sourceModule).select(tree.symbol)
case TermRef(thiz: ThisType, _) =>
assert(tree.symbol.is(Flags.JavaStatic))
tree
case _ => tree
}
}
Expand Down
16 changes: 12 additions & 4 deletions src/dotty/tools/dotc/transform/LambdaLift.scala
Original file line number Diff line number Diff line change
Expand Up @@ -249,14 +249,21 @@ class LambdaLift extends MiniPhase with IdentityDenotTransformer { thisTransform
else if (sym is Method) markCalled(sym, enclosure)
else if (sym.isTerm) markFree(sym, enclosure)
}
if (sym.maybeOwner.isClass) narrowTo(sym.owner.asClass)
def captureImplicitThis(x: Type): Unit = {
x match {
case tr@TermRef(x, _) if (!tr.termSymbol.isStatic) => captureImplicitThis(x)
case x: ThisType if (!x.tref.typeSymbol.isStaticOwner) => narrowTo(x.tref.typeSymbol.asClass)
case _ =>
}
}
captureImplicitThis(tree.tpe)
case tree: Select =>
if (sym.is(Method) && isLocal(sym)) markCalled(sym, enclosure)
case tree: This =>
narrowTo(tree.symbol.asClass)
case tree: DefDef =>
if (sym.owner.isTerm && !sym.is(Label))
liftedOwner(sym) = sym.enclosingClass.topLevelClass
liftedOwner(sym) = sym.enclosingPackageClass
// this will make methods in supercall constructors of top-level classes owned
// by the enclosing package, which means they will be static.
// On the other hand, all other methods will be indirectly owned by their
Expand Down Expand Up @@ -362,8 +369,9 @@ class LambdaLift extends MiniPhase with IdentityDenotTransformer { thisTransform
// though the second condition seems weird, it's not true for symbols which are defined in some
// weird combinations of super calls.
(encClass, EmptyFlags)
} else
(topClass, JavaStatic)
} else if (encClass.is(ModuleClass, butNot = Package) && encClass.isStatic) // needed to not cause deadlocks in classloader. see t5375.scala
(encClass, EmptyFlags)
else (topClass, JavaStatic)
}
else (lOwner, EmptyFlags)
local.copySymDenotation(
Expand Down
11 changes: 11 additions & 0 deletions tests/run/t5375.scala
Original file line number Diff line number Diff line change
@@ -1,3 +1,14 @@
/** Hello fellow compiler developer.
if you are wondering why does test suite hang on this test
then it's likely that the lambda inside map has been compiled into static method
unfotrunatelly, as it is executed inside static object initializer,
it is executed inside class-loader, in a synchronized block that is not source defined.

If the lambda will be static Test$#foo, calling it through a different thread would require grabbing the
lock inside classloader. Unlike if it not static and is called through This(Test).foo, no lock is grabbed.

@DarkDimius
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:-)

object Test extends dotty.runtime.LegacyApp {
val foos = (1 to 1000).toSeq
try
Expand Down