Skip to content

Use proxy-based serialization for modules #7297

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 1 commit into from
Oct 18, 2018

Conversation

retronym
Copy link
Member

@retronym retronym commented Oct 1, 2018

This avoids serializing any state of serializable singletons which
would be discarded anyway after deserializing (SI-10412).

Use ClassValue to cache instances, which is significantly faster for
subsequent deserialization of a given module. ClassValue entries only
weakly reference the key and value so the class can be unloaded.

Wrap use of reflection in AccessControler to play nicely under
SecurityManager. A security manager will only walk up the stack
to this point to check permissions, so if users grant scala-library
permissions in the policy, the call will be allowed regardless of
the code higher in the stack that has triggered deserialization.

(Rebase and extension of #6877)

Fixes scala/bug#10412

Copy link
Member

@lrytz lrytz left a comment

Choose a reason for hiding this comment

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

Nice!

@mkeskells
Copy link
Contributor

Should this be marked [nomerge] as we cant apply this to 2.12 because of serialisation binary compat
Not sure of these special tags though

@mkeskells
Copy link
Contributor

mkeskells commented Oct 3, 2018

As we are asserting that this work well with a SecurityManager, with minimal permissions (i.e. Scala.* has reflect access), there should be a test for that

@retronym retronym self-assigned this Oct 17, 2018
@retronym
Copy link
Member Author

I'll rebase and improve the comment.

@retronym retronym force-pushed the review/6877 branch 2 times, most recently from 8976c9b to f3f9ab5 Compare October 18, 2018 01:43
This avoids serializing any state of serializable singletons which
would be discarded anyway after deserializing (SI-10412).

Use ClassValue to cache instances, which is significantly faster for
subsequent deserialization of a given module. ClassValue entries only
weakly reference the key and value so the class can be unloaded.

Wrap use of reflection in AccessControler to play nicely under
SecurityManager. A security manager will only walk up the stack
to this point to check permissions, so if users grant scala-library
permissions in the policy, the call will be allowed regardless of
the code higher in the stack that has triggered deserialization.
@retronym
Copy link
Member Author

retronym commented Oct 18, 2018

@mkeskells

As we are asserting that this work well with a SecurityManager, with minimal permissions (i.e. Scala.* has reflect access), there should be a test for that

I can't really figure out how to (or maybe, muster the energy to) rig up a test for this. I think it would require me to run a forked JVM with a security manager and a setting for -Dpackage.access=com.foo.bar, define an object in com.foo.bar, call readResolve from a class in an unprivileged protection domain.

Should this be marked [nomerge] as we cant apply this to 2.12 because of serialisation binary compat. Not sure of these special tags though

That's only required for changes committed to 2.12.x that shouldn't/needn't be merged forward to 2.13.x.

@mkeskells
Copy link
Contributor

@retronym I don't think that it is as complex as that for the test. I will pop one in a separate new PR

great to see this merged 😄

smarter pushed a commit to dotty-staging/dotty that referenced this pull request Jan 22, 2019
In scala#4450 and Scala 2.12, readResolve is used to make sure deserializing
object returns the singleton instance of the object, but this doesn't
prevent the fields of the objects from being serializable 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>
smarter pushed a commit to dotty-staging/dotty that referenced this pull request Jan 22, 2019
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>
smarter pushed a commit to dotty-staging/dotty that referenced this pull request Jan 22, 2019
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>
smarter pushed a commit to dotty-staging/dotty that referenced this pull request Jan 22, 2019
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>
smarter pushed a commit to smarter/dotty that referenced this pull request Jan 25, 2019
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>
smarter pushed a commit to dotty-staging/dotty that referenced this pull request Jan 25, 2019
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>
smarter pushed a commit to dotty-staging/dotty that referenced this pull request Jan 25, 2019
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>
smarter pushed a commit to dotty-staging/dotty that referenced this pull request Jan 25, 2019
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>
smarter added a commit to dotty-staging/dotty that referenced this pull request Jan 25, 2019
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>
smarter added a commit to dotty-staging/dotty that referenced this pull request Jan 26, 2019
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>
smarter added a commit to dotty-staging/dotty that referenced this pull request Feb 2, 2019
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>
smarter added a commit to dotty-staging/dotty that referenced this pull request Feb 2, 2019
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants