Test & fix for regression in Lazy serialization uncovered by #2325#10
Merged
gkossakowski merged 2 commits intogkossakowski:nameHashing/private-membersfrom Dec 22, 2015
Conversation
Author
|
note that |
13ef52a to
48ded49
Compare
The crash manifested as a `NotSerializableException`, because we don't visit the private subtrees of the API graph, which would previously force all Lazy stubs as a side-effect. There was a backstop in `AbstractLazy` (`writeReplace`), but somehow that didn't work. ``` java.io.NotSerializableException: sun.reflect.generics.reflectiveObjects.ParameterizedTypeImpl [...] at java.io.ObjectOutputStream.writeObject(ObjectOutputStream.java:329) at xsbt.api.SourceFormat$.writes(SourceFormat.scala:24) at xsbt.api.SourceFormat$.writes(SourceFormat.scala:15) at sbt.inc.TextAnalysisFormat21661anonfun.apply(TextAnalysisFormat.scala:329) at sbt.inc.TextAnalysisFormat21661anonfun.apply(TextAnalysisFormat.scala:329) at sbt.inc.TextAnalysisFormat21661anonfun at java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.javr$.aggregate(TextAnalysisFormat.scala:18) at sbt.inc.TextAnalysisFormat$.objToString(TextAnalysisFormat.scala:329) at sbt.inc.TextAnalysisFormat21661anonfun.apply(TextAnalysisFormat.scala:213) at sbt.inc.TextAnalysisFormat21661anonfun.apply(TextAnalysisFormat.scala:213) at sbt.inc.TextAnalysisFormat21661anonfun21661writeMap.apply(TextAnalysisFormat.scala:381) at sbt.inc.TextAnalysisFormat21661anonfun21661writeMap.apply(TextAnalysisFormat.scala:377) at scala.collection.mutable.ResizableArray.foreach(Resizable at sbt.inc.TextAnalysisFormalection.mutable.ArrayBuffer.foreach(ArrayBuffer.scala:47) at sbt.inc.TextAnalysisFormat$.sbt21661writeMap(TextAnalysisFormat.scala:377) at sbt.inc.TextAnalysisFormat$.write(TextAnalysisFormat.scala:216) at sbt.inc.TextAnalysisFormat21661anonfun.apply(TextAnalysisFormat.scala:64) at sbt.inc.TextAnalysisFormat21661anonfun.apply(TextAnalysisFormat.scala:64) at sbt.inc.TextAnalysisFormat21661anonfun.apply(TextAnalysisFormat.scala:64) at sbt.inc.FormatTimer$.aggregate(TextAnalysisFo at sbt.inc.TextAnalysisFormat21661anonfun.scala:25) at sbt.inc.TextAnalysisFormat$.write(TextAnalysisFormat.scala:64) at sbt.inc.FileBasedStore21661anon21661anonfun.apply(FileBasedStore.scala:12) at sbt.inc.FileBasedStore21661anon21661anonfun.apply(FileBasedStore.scala:12) at sbt.Using.apply(Using.scala:24) at sbt.inc.FileBasedStore21661anon.set(FileBasedStore.scala:12) at sbt.inc.AnalysisStore21661anon.set(AnalysisStore.scala:16) at sbt.inc.AnalysisStore21661anon.set(AnalysisStore.scala:27) at sbt.Defaults21661anonfun.apply(Defaults.scala:799) at sbt.Defaults21661anonfun at sbt.inc.TextAts.scala:793) at scala.Function121661anonfun.apply(Function1.scala:47) at sbt.21661anonfun21661u2219.apply(TypeFunctions.scala:40) at sbt.std.Transform21661anon.work(System.scala:63) at sbt.Execute21661anonfun21661anonfun.apply(Execute.scala:226) at sbt.Execute21661anonfun21661anonfun.apply(Execute.scala:226) at sbt.ErrorHandling$.wideConvert(ErrorHandling.scala:17) at sbt.Execute.work(Execute.scala:235) at sbt.Execute21661anonfun.apply(Exe at sbt.Using.apply(Using.scala:24) mit.apply(Execute.scala:226) at sbt.ConcurrentRestrictions21661anon21661anonfun.apply(ConcurrentRestrictions.scala:159) at sbt.CompletionService21661anon.call(CompletionService.scala:28) at java.util.concurrent.FutureTask.innerRun(FutureTask.java:303) at java.util.concurrent.FutureTask.run(FutureTask.java:138) at java.util.concurrent.Executors.call(Executors.java:439) at java.util.concurrent.FutureTask.innerRun(FutureTask.java:303 at sbt.std.Transform21661anon.work(System(FutureTask.java:138) at java.util.concurrent.ThreadPoolExecutor.runTask(ThreadPoolExecutor.java:895) at java.util.concurrent.ThreadPoolExecutor.run(ThreadPoolExecutor.java:918) at java.lang.Thread.run(Thread.java:695) [error] (compile:compile) java.io.NotSerializableException: sun.reflect.generics.reflectiveObjects.ParameterizedTypeImpl [error] Total time: 2 s, completed Dec 21, 2015 12:05:19 PM ```
Fix for regression triggered by sbt#2325 -- apparently, the API visitor would force all the lazy stubs so the `AbstractLazy` `writeReplace` was not exercised. With the private subtrees being ignored, the visitor didn't force the lazy stub, and the `writeProtected` method was not inherited in `SafeLazy.Impl`. From https://docs.oracle.com/javase/7/docs/api/java/io/Serializable.html `writeReplace` must be protected for its override to be inherited. > `Serializable` classes that need to designate an alternative object to be used > when writing an object to the stream should implement this special method with > the exact signature: > `ANY-ACCESS-MODIFIER Object writeReplace() throws ObjectStreamException;` > > This `writeReplace` method is invoked by serialization if the method exists and > it would be accessible from a method defined within the class of the object > being serialized. Thus, the method can have `private`, `protected` and > `package-private` access. > > **Subclass access to this method follows java accessibility rules.** (Thanks to retronym for digging up the docs.) The fix is captured, indirectly, by the scripted test `source-dependencies/java-analysis-serialization-error`.
48ded49 to
95da6ec
Compare
Owner
|
apiDebug disables shrinking of api objects so you have something to print. Otherwise only hash code is kept. The api extraction from Scala compiler forces all lazy structures before returning api: Maybe we could do the same for Java api extraction? I'll check and report back. |
Author
|
I believe I have fixed the issue in the 95da6ec commit above. |
Owner
|
You're right. Thanks! |
gkossakowski
added a commit
that referenced
this pull request
Dec 22, 2015
…-members Test & fix for regression in Lazy serialization uncovered by sbt#2325
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
The crash manifested as a
NotSerializableException,because we don't visit the private subtrees of the API graph,
which would previously force all Lazy stubs as a side-effect.
There was a backstop in
AbstractLazy(writeReplace), butsomehow that didn't work.