Fix #4440: Do not serialize the content of static objects #5775
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dottybot
reviewed
Jan 22, 2019
smarter
force-pushed
the
write-replace
branch
3 times, most recently
from
fa96adf
to
9e7a762
Compare
|
@retronym feel free to review and let us know if there are relevant test cases from scalac we should add. |
smarter
added a commit
to smarter/dotty
that referenced
this pull request
Jan 25, 2019
To avoid deadlocks when combining objects, lambdas and multi-threading, lambdas in objects are compiled to instance methods of the module class instead of static methods (see tests/run/deadlock.scala and scala/scala-dev#195 for details). This has worked well for us so far but this is problematic for serialization: serializing a lambda requires serializing all the values it captures, if this lambda is in an object, this means serializing the enclosing object, which fails if the object does not extend Serializable. Because serializing objects is basically free since scala#5775, it seems like the simplest solution is to simply make all objects Serializable, this certainly seems preferable to deadlocks.
smarter
force-pushed
the
write-replace
branch
from
9e7a762
to
0735ca0
Compare
smarter
added a commit
to smarter/dotty
that referenced
this pull request
Jan 25, 2019
To avoid deadlocks when combining objects, lambdas and multi-threading, lambdas in objects are compiled to instance methods of the module class instead of static methods (see tests/run/deadlock.scala and scala/scala-dev#195 for details). This has worked well for us so far but this is problematic for serialization: serializing a lambda requires serializing all the values it captures, if this lambda is in an object, this means serializing the enclosing object, which fails if the object does not extend Serializable. Because serializing objects is basically free since scala#5775, it seems like the simplest solution is to simply make all objects Serializable, this certainly seems preferable to deadlocks.
smarter
force-pushed
the
write-replace
branch
from
0735ca0
to
eb8e694
Compare
smarter
added a commit
to smarter/dotty
that referenced
this pull request
Jan 25, 2019
To avoid deadlocks when combining objects, lambdas and multi-threading, lambdas in objects are compiled to instance methods of the module class instead of static methods (see tests/run/deadlock.scala and scala/scala-dev#195 for details). This has worked well for us so far but this is problematic for serialization: serializing a lambda requires serializing all the values it captures, if this lambda is in an object, this means serializing the enclosing object, which fails if the object does not extend Serializable. Because serializing objects is basically free since scala#5775, it seems like the simplest solution is to simply make all objects Serializable, this certainly seems preferable to deadlocks.
smarter
force-pushed
the
write-replace
branch
2 times, most recently
from
5db7456
to
3b9ed7f
Compare
smarter
added a commit
to smarter/dotty
that referenced
this pull request
Jan 25, 2019
To avoid deadlocks when combining objects, lambdas and multi-threading, lambdas in objects are compiled to instance methods of the module class instead of static methods (see tests/run/deadlock.scala and scala/scala-dev#195 for details). This has worked well for us so far but this is problematic for serialization: serializing a lambda requires serializing all the values it captures, if this lambda is in an object, this means serializing the enclosing object, which fails if the object does not extend Serializable. Because serializing objects is basically free since scala#5775, it seems like the simplest solution is to simply make all objects Serializable, this certainly seems preferable to deadlocks.
smarter
force-pushed
the
write-replace
branch
from
3b9ed7f
to
03ed10e
Compare
smarter
added a commit
to smarter/dotty
that referenced
this pull request
Jan 26, 2019
To avoid deadlocks when combining objects, lambdas and multi-threading, lambdas in objects are compiled to instance methods of the module class instead of static methods (see tests/run/deadlock.scala and scala/scala-dev#195 for details). This has worked well for us so far but this is problematic for serialization: serializing a lambda requires serializing all the values it captures, if this lambda is in an object, this means serializing the enclosing object, which fails if the object does not extend Serializable. Because serializing objects is basically free since scala#5775, it seems like the simplest solution is to simply make all objects Serializable, this certainly seems preferable to deadlocks.
smarter
added a commit
to smarter/dotty
that referenced
this pull request
Jan 26, 2019
To avoid deadlocks when combining objects, lambdas and multi-threading, lambdas in objects are compiled to instance methods of the module class instead of static methods (see tests/run/deadlock.scala and scala/scala-dev#195 for details). This has worked well for us so far but this is problematic for serialization: serializing a lambda requires serializing all the values it captures, if this lambda is in an object, this means serializing the enclosing object, which fails if the object does not extend Serializable. Because serializing objects is basically free since scala#5775, it seems like the simplest solution is to simply make all objects Serializable, this certainly seems preferable to deadlocks.
smarter
force-pushed
the
write-replace
branch
from
3149a0c
to
91574bd
Compare
|
@nicolasstucki I just added support for |
|
I will have a look |
smarter
commented
Jan 26, 2019
|
Now the tests pass |
OlivierBlanvillain
approved these changes
Jan 31, 2019
smarter
force-pushed
the
write-replace
branch
from
11a7889
to
07a5961
Compare
In scala#4450 and Scala 2.12, readResolve is used to make sure deserializing an object returns the singleton instance of the object, but this doesn't prevent the fields of the objects from being serialized in the first place even though they're not used. Scala 2.13 switched to using writeReplace to completely bypass serialization of the object in scala/scala#7297. This commit adapts this to Dotty. Co-Authored-By: Ingar Abrahamsen <ingar@abrahams1.com> Co-Authored-By: Jason Zaugg <jzaugg@gmail.com>
And use this in SyntheticMethods instead of the impossible-to-write-in-user-code `classOf[Foo$]`. This is necessary if we want decompilation to produce valid source code for objects. Note that the decompilation printer is currently wrong here and will print `classOf[Foo]` instead of `classOf[Foo.type]`.
smarter
force-pushed
the
write-replace
branch
from
07a5961
to
95900de
Compare
smarter
added a commit
to smarter/dotty
that referenced
this pull request
Feb 2, 2019
To avoid deadlocks when combining objects, lambdas and multi-threading, lambdas in objects are compiled to instance methods of the module class instead of static methods (see tests/run/deadlock.scala and scala/scala-dev#195 for details). This has worked well for us so far but this is problematic for serialization: serializing a lambda requires serializing all the values it captures, if this lambda is in an object, this means serializing the enclosing object, which fails if the object does not extend Serializable. Because serializing objects is basically free since scala#5775, it seems like the simplest solution is to simply make all objects Serializable, this certainly seems preferable to deadlocks.
smarter
added a commit
to dotty-staging/dotty
that referenced
this pull request
Feb 3, 2019
To avoid deadlocks when combining objects, lambdas and multi-threading, lambdas in objects are compiled to instance methods of the module class instead of static methods (see tests/run/deadlock.scala and scala/scala-dev#195 for details). This has worked well for us so far but this is problematic for serialization: serializing a lambda requires serializing all the values it captures, if this lambda is in an object, this means serializing the enclosing object, which fails if the object does not extend Serializable. Because serializing objects is basically free since scala#5775, it seems like the simplest solution is to simply make all objects Serializable, this certainly seems preferable to deadlocks. For some reason, this commit causes the issue described in scala#3383 to reappear, we add a workaround for that in Trees.scala.
smarter
added a commit
to dotty-staging/dotty
that referenced
this pull request
Feb 3, 2019
To avoid deadlocks when combining objects, lambdas and multi-threading, lambdas in objects are compiled to instance methods of the module class instead of static methods (see tests/run/deadlock.scala and scala/scala-dev#195 for details). This has worked well for us so far but this is problematic for serialization: serializing a lambda requires serializing all the values it captures, if this lambda is in an object, this means serializing the enclosing object, which fails if the object does not extend Serializable. Because serializing objects is basically free since scala#5775, it seems like the simplest solution is to simply make all objects Serializable, this certainly seems preferable to deadlocks. For some reason, this commit causes the issue described in scala#3383 to reappear, we add a workaround for that in Trees.scala.
smarter
added a commit
to dotty-staging/dotty
that referenced
this pull request
Feb 3, 2019
To avoid deadlocks when combining objects, lambdas and multi-threading, lambdas in objects are compiled to instance methods of the module class instead of static methods (see tests/run/deadlock.scala and scala/scala-dev#195 for details). This has worked well for us so far but this is problematic for serialization: serializing a lambda requires serializing all the values it captures, if this lambda is in an object, this means serializing the enclosing object, which fails if the object does not extend Serializable. Because serializing objects is basically free since scala#5775, it seems like the simplest solution is to simply make all objects Serializable, this certainly seems preferable to deadlocks. This commit causes a cyclic reference to happen in some cases, we add a workaround to avoid this in Trees.scala and fix it properly in the commit.
smarter
added a commit
to dotty-staging/dotty
that referenced
this pull request
Feb 6, 2019
To avoid deadlocks when combining objects, lambdas and multi-threading, lambdas in objects are compiled to instance methods of the module class instead of static methods (see tests/run/deadlock.scala and scala/scala-dev#195 for details). This has worked well for us so far but this is problematic for serialization: serializing a lambda requires serializing all the values it captures, if this lambda is in an object, this means serializing the enclosing object, which fails if the object does not extend Serializable. Because serializing objects is basically free since scala#5775, it seems like the simplest solution is to simply make all objects Serializable, this certainly seems preferable to deadlocks. This commit causes a cyclic reference to happen in some cases, we add a workaround to avoid this in Trees.scala and fix it properly in the commit.
smarter
added a commit
to dotty-staging/dotty
that referenced
this pull request
Feb 6, 2019
To avoid deadlocks when combining objects, lambdas and multi-threading, lambdas in objects are compiled to instance methods of the module class instead of static methods (see tests/run/deadlock.scala and scala/scala-dev#195 for details). This has worked well for us so far but this is problematic for serialization: serializing a lambda requires serializing all the values it captures, if this lambda is in an object, this means serializing the enclosing object, which fails if the object does not extend Serializable. Because serializing objects is basically free since scala#5775, it seems like the simplest solution is to simply make all objects Serializable, this certainly seems preferable to deadlocks. This commit causes a cyclic reference to happen in some cases, we add a workaround to avoid this in Trees.scala and fix it properly in the commit.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In #4450 and Scala 2.12, readResolve is used to make sure deserializing
an object returns the singleton instance of the object, but this doesn't
prevent the fields of the objects from being serialized in the first
place even though they're not used. Scala 2.13 switched to using
writeReplace to completely bypass serialization of the object in
scala/scala#7297. This commit adapts this to
Dotty.
Co-Authored-By: Ingar Abrahamsen ingar@abrahams1.com
Co-Authored-By: Jason Zaugg jzaugg@gmail.com