-
Notifications
You must be signed in to change notification settings - Fork 245
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
Resolves issue 323 #324
base: JDK-8
Are you sure you want to change the base?
Resolves issue 323 #324
Conversation
deserialization error that I have been experiencing.
DefaultFSTInt2ObjectMap's clear method not setting next to null. When the map was renamed to DefaultFSTInt2ObjectMap and an option of passing in your own map was added the clear method was also optimized and I think this was just missed.
to make more sense on the JDK-8 branch. I also changed how the javadoc doclint is disabled so that I could build the jar.
@@ -78,7 +78,7 @@ | |||
<groupId>org.apache.maven.plugins</groupId> | |||
<artifactId>maven-javadoc-plugin</artifactId> | |||
<configuration> | |||
<additionalparam>-Xdoclint:none</additionalparam> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know Gradle a lot better than Maven. I'm happy to remove these changes if you don't want them.
@@ -172,6 +172,7 @@ public void clear() { | |||
mKeys = new int[size]; | |||
mValues = new Object[size]; | |||
mNumberOfElements = 0; | |||
next = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the most important line that is needed and which fixes the issue.
import deserialization_cache_bug.random.DeterministicRandomGenerator; | ||
import deserialization_cache_bug.random.RandomGenerator; | ||
|
||
public class DeserializationCacheTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is pretty complex, but I had a really hard time reproducing the issue and tried to mimic some of the things about our complex model that might make it more likely to happen.
Any news on when this might get merged into master and released? |
See #323 for details. This PR includes:
1 - Some pom.xml updates that set it to use Java 8 and fix a javadoc issue that kept me from building the jar file.
2 - A complex test that can deterministically reproduce the bug.
3 - The simple one liner fix for the issue.
The fix would be good to get into other branches as well.