SD-120 Non FunctionN lambdas should not be universally serializable by retronym · Pull Request #5278 · scala/scala

retronym · 2016-07-14T01:05:49Z

Instead, we follow the example set by javac, and predicate serializability
of bot anon-class and invokedynamic-based lambdas on whether or not the
SAM type extends java.io.Serializable.

Fixes scala/scala-dev#120

retronym · 2016-07-14T01:07:23Z

Review by @adriaanm @lrytz.

The argument for this change is "do as javac does". It is worth considering the counterargument: "do as scalac already does for FunctionN-based lambdas".

lrytz · 2016-07-21T05:56:46Z

src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala

+    val bridgeCount = 0
+    import java.lang.invoke.LambdaMetafactory.{FLAG_MARKERS, FLAG_SERIALIZABLE}
+    def flagIf(b: Boolean, flag: Int): Int = if (b) flag else 0
+    val flags = flagIf(markerInterfaces.isEmpty, FLAG_MARKERS) | flagIf(serializable, FLAG_SERIALIZABLE)


markerInterfaces.nonEmpty?

Weird, now I don't trust my test 😦

Oh, we can just always provide that flag, even if we have marker count of 0.

Instead, we follow the example set by javac, and predicate serializability of bot anon-class and invokedynamic-based lambdas on whether or not the SAM type extends java.io.Serializable. Fixes scala/scala-dev#120

retronym · 2016-07-22T00:29:27Z

@lrytz review comments addressed and rebased.

lrytz · 2016-07-22T06:59:47Z

LGTM

lrytz · 2016-07-22T07:07:40Z

Actually, I'm wondering now: for SAM types that extend java.io.Serializable, we don't add the FLAG_SERIALIZABLE, we only use that flag for FunctionN which are not serializable.

Comparing that to Java, when does javac set the FLAG_SERIALIZABLE? You say that javac only makes closures serializable if the SAM type is serializable. Does it use the flag in this case? Then maybe we should do the same?

retronym · 2016-07-22T08:38:13Z

That's what I mean to do, I'll double check and submit a follow up PR to
set that flag and to test that such a Sam serialises via SerializedLambda
On Fri, 22 Jul 2016 at 17:07, Lukas Rytz notifications@github.com wrote:

Actually, I'm wondering now: for SAM types that extend
java.io.Serializable, we don't add the FLAG_SERIALIZABLE, we only use
that flag for FunctionN which are not serializable.

Comparing that to Java, when does javac set the FLAG_SERIALIZABLE? You
say that javac only makes closures serializable if the SAM type is
serializable. Does it use the flag in this case? Then maybe we should do
the same?

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#5278 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAEAD0mA4ol8TWPRe3TAvosK12QnDcATks5qYGw-gaJpZM4JL_k6
.

retronym · 2016-07-25T00:42:27Z

We do set the SERIALIZABLE flag for non FunctionN lambdas, given that the functional interface itself extends java.io.Serializable.

scala> trait T extends java.io.Serializable { def apply(): Int }
defined trait T

scala> class Sammy { def t: T = () => 42 }
defined class Sammy

scala> def callWriteReplace(a: AnyRef) = { val wr = a.getClass.getDeclaredMethod("writeReplace"); wr.setAccessible(true); wr.invoke(a) }
callWriteReplace: (a: AnyRef)Object

scala> callWriteReplace(new Sammy().t)
res6: Object = SerializedLambda[capturingClass=class Sammy, functionalInterfaceMethod=T.apply:()I, implementation=invokeStatic Sammy.$anonfun$t$1:()I, instantiatedMethodType=()I, numCaptured=0]

scala> :javap Sammy
...
BootstrapMethods:
  0: #36 invokestatic java/lang/invoke/LambdaMetafactory.altMetafactory:(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;[Ljava/lang/Object;)Ljava/lang/invoke/CallSite;
    Method arguments:
      #38 ()I
      #42 invokestatic $line8/$read$$iw$$iw$Sammy.$anonfun$t$1:()I
      #38 ()I
      #43 3 // flags
      #44 1
      #46 scala/Serializable
      #47 0
  1: #65 invokestatic scala/runtime/LambdaDeserialize.bootstrap:(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;)Ljava/lang/invoke/CallSite;
    Method arguments:

scala> FLAG_SERIALIZABLE | FLAG_MARKERS
res7: Int = 3

retronym · 2016-07-25T00:51:24Z

import scala.util.Try

trait T1 { def apply(): Int }
trait T2 extends java.io.Serializable { def apply(): Int }

class Sammy {
  def f0     = () => 0
  def t1: T1 = () => 1
  def t2: T2 = () => 2
}

object Test {
   def callWriteReplace(a: AnyRef) = { val wr = a.getClass.getDeclaredMethod("writeReplace"); wr.setAccessible(true); wr.invoke(a) }

   def main(args: Array[String]) {
     val s = new Sammy()
     println(Try(callWriteReplace(s.f0)))
     println(Try(callWriteReplace(s.t1)))
     println(Try(callWriteReplace(s.t2)))
   }
}

% qscalac sandbox/test.scala && qscala Test && javap -v Sammy
Success(SerializedLambda[capturingClass=class Sammy, functionalInterfaceMethod=scala/runtime/java8/JFunction0$mcI$sp.apply$mcI$sp:()I, implementation=invokeStatic Sammy.$anonfun$f0$1:()I, instantiatedMethodType=()I, numCaptured=0])
Failure(java.lang.NoSuchMethodException: Sammy$$Lambda$127/123961122.writeReplace())
Success(SerializedLambda[capturingClass=class Sammy, functionalInterfaceMethod=T2.apply:()I, implementation=invokeStatic Sammy.$anonfun$t2$1:()I, instantiatedMethodType=()I, numCaptured=0])


...
{
  public scala.Function0<java.lang.Object> f0();
    descriptor: ()Lscala/Function0;
    flags: ACC_PUBLIC
    Code:
      stack=1, locals=1, args_size=1
         0: invokedynamic #36,  0             // InvokeDynamic #0:apply$mcI$sp:()Lscala/runtime/java8/JFunction0$mcI$sp;
         5: areturn
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            0       6     0  this   LSammy;
      LineNumberTable:
        line 7: 0
    Signature: #78                          // ()Lscala/Function0<Ljava/lang/Object;>;

  public T1 t1();
    descriptor: ()LT1;
    flags: ACC_PUBLIC
    Code:
      stack=1, locals=1, args_size=1
         0: invokedynamic #49,  0             // InvokeDynamic #1:apply:()LT1;
         5: areturn
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            0       6     0  this   LSammy;
      LineNumberTable:
        line 8: 0

  public T2 t2();
    descriptor: ()LT2;
    flags: ACC_PUBLIC
    Code:
      stack=1, locals=1, args_size=1
         0: invokedynamic #57,  0             // InvokeDynamic #2:apply:()LT2;
         5: areturn
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            0       6     0  this   LSammy;
      LineNumberTable:
        line 9: 0

  public static final int $anonfun$f0$1();
    descriptor: ()I
    flags: ACC_PUBLIC, ACC_STATIC, ACC_FINAL, ACC_SYNTHETIC
    Code:
      stack=1, locals=0, args_size=0
         0: iconst_0
         1: ireturn
      LineNumberTable:
        line 7: 0

  public static final int $anonfun$t1$1();
    descriptor: ()I
    flags: ACC_PUBLIC, ACC_STATIC, ACC_FINAL, ACC_SYNTHETIC
    Code:
      stack=1, locals=0, args_size=0
         0: iconst_1
         1: ireturn
      LineNumberTable:
        line 8: 0

  public static final int $anonfun$t2$1();
    descriptor: ()I
    flags: ACC_PUBLIC, ACC_STATIC, ACC_FINAL, ACC_SYNTHETIC
    Code:
      stack=1, locals=0, args_size=0
         0: iconst_2
         1: ireturn
      LineNumberTable:
        line 9: 0

  public Sammy();
    descriptor: ()V
    flags: ACC_PUBLIC
    Code:
      stack=1, locals=1, args_size=1
         0: aload_0
         1: invokespecial #61                 // Method java/lang/Object."<init>":()V
         4: return
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            0       5     0  this   LSammy;
      LineNumberTable:
        line 12: 0
        line 6: 4
}
BootstrapMethods:
  0: #22 invokestatic java/lang/invoke/LambdaMetafactory.altMetafactory:(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;[Ljava/lang/Object;)Ljava/lang/invoke/CallSite;
    Method arguments:
      #24 ()I
      #28 invokestatic Sammy.$anonfun$f0$1:()I
      #24 ()I
      #29 3
      #30 1
      #32 scala/Serializable
  1: #22 invokestatic java/lang/invoke/LambdaMetafactory.altMetafactory:(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;[Ljava/lang/Object;)Ljava/lang/invoke/CallSite;
    Method arguments:
      #24 ()I
      #44 invokestatic Sammy.$anonfun$t1$1:()I
      #24 ()I
      #45 2
      #46 0
  2: #22 invokestatic java/lang/invoke/LambdaMetafactory.altMetafactory:(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;[Ljava/lang/Object;)Ljava/lang/invoke/CallSite;
    Method arguments:
      #24 ()I
      #55 invokestatic Sammy.$anonfun$t2$1:()I
      #24 ()I
      #29 3
      #46 0
  3: #70 invokestatic scala/runtime/LambdaDeserialize.bootstrap:(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;)Ljava/lang/invoke/CallSite;
    Method arguments:
SourceFile: "test.scala"
InnerClasses:
     public static final #13= #10 of #12; //Lookup=class java/lang/invoke/MethodHandles$Lookup of class java/lang/invoke/MethodHandles
RuntimeVisibleAnnotations:
  0: #6(#7=s#8)
Error: unknown attribute
  ScalaInlineInfo: length = 0x27
   01 00 00 07 00 19 00 17 01 00 29 00 17 01 00 34
   00 17 01 00 3A 00 3B 00 00 0E 00 0F 00 00 27 00
   28 00 00 32 00 33 00
Error: unknown attribute
  ScalaSig: length = 0x3
   05 00 00

lrytz · 2016-08-08T13:49:40Z

ok, thanks. i double-checked again, i confused the meanings of the isSerializable and addScalaSerializableMarker flags - the former is for the flag, the latter for adding the parent interface.

Infrastructure taken from scala/scala#5278

Test infrastructure taken from scala/scala#5278

retronym force-pushed the ticket/SD-120 branch from 76a9640 to 3adf8a4 Compare July 14, 2016 02:05

retronym added this to the 2.12.0-RC1 milestone Jul 14, 2016

retronym assigned adriaanm Jul 14, 2016

lrytz reviewed Jul 21, 2016
View reviewed changes

retronym force-pushed the ticket/SD-120 branch 2 times, most recently from 583c474 to 551beaf Compare July 22, 2016 00:27

SD-120 Non FunctionN lambdas should not be universally serializable

3e64fdd

Instead, we follow the example set by javac, and predicate serializability of bot anon-class and invokedynamic-based lambdas on whether or not the SAM type extends java.io.Serializable. Fixes scala/scala-dev#120

retronym force-pushed the ticket/SD-120 branch from 551beaf to 3e64fdd Compare July 22, 2016 00:29

retronym assigned lrytz Jul 22, 2016

scala-jenkins added the reviewed label Jul 22, 2016

lrytz merged commit 8d87b62 into scala:2.12.x Jul 22, 2016

adriaanm added the 2.12 label Oct 29, 2016

adriaanm mentioned this pull request Nov 29, 2016

Test documenting LineNumber behaviour on Scala 2.11, 2.12, Java 8 akka/akka-core#21897

Merged

OlivierBlanvillain added a commit to dotty-staging/dotty that referenced this pull request Jan 17, 2019

Test actual serialization

7f670df

Infrastructure taken from scala/scala#5278

smarter pushed a commit to dotty-staging/dotty that referenced this pull request Jan 19, 2019

Make case classes Serializable

c5dcbc6

Test infrastructure taken from scala/scala#5278

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SD-120 Non FunctionN lambdas should not be universally serializable#5278

SD-120 Non FunctionN lambdas should not be universally serializable#5278
lrytz merged 1 commit intoscala:2.12.xfrom
retronym:ticket/SD-120

retronym commented Jul 14, 2016 •

edited

Loading

Uh oh!

retronym commented Jul 14, 2016 •

edited

Loading

Uh oh!

lrytz Jul 21, 2016

Uh oh!

retronym Jul 21, 2016

Uh oh!

retronym Jul 22, 2016

Uh oh!

retronym commented Jul 22, 2016

Uh oh!

lrytz commented Jul 22, 2016

Uh oh!

lrytz commented Jul 22, 2016

Uh oh!

retronym commented Jul 22, 2016

Uh oh!

retronym commented Jul 25, 2016

Uh oh!

retronym commented Jul 25, 2016

Uh oh!

lrytz commented Aug 8, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

retronym commented Jul 14, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

retronym commented Jul 14, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lrytz Jul 21, 2016

Choose a reason for hiding this comment

Uh oh!

retronym Jul 21, 2016

Choose a reason for hiding this comment

Uh oh!

retronym Jul 22, 2016

Choose a reason for hiding this comment

Uh oh!

retronym commented Jul 22, 2016

Uh oh!

lrytz commented Jul 22, 2016

Uh oh!

lrytz commented Jul 22, 2016

Uh oh!

retronym commented Jul 22, 2016

Uh oh!

retronym commented Jul 25, 2016

Uh oh!

retronym commented Jul 25, 2016

Uh oh!

lrytz commented Aug 8, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

retronym commented Jul 14, 2016 •

edited

Loading

retronym commented Jul 14, 2016 •

edited

Loading