Skip to content
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

CachedFields are not reset when references-flag changes #973

Open
thomas-z96 opened this issue May 5, 2023 · 3 comments
Open

CachedFields are not reset when references-flag changes #973

thomas-z96 opened this issue May 5, 2023 · 3 comments
Labels

Comments

@thomas-z96
Copy link

Describe the bug
When changing the references flag from false to true on a kryo instance that has already been used for deserializing, subsequent deserialization does not work correctly any more (wrong results / exceptions).

To Reproduce

@Test
public void testBug() {
    Output output1 = new Output(512, -1);
    Kryo kryo = getKryoInstance(false);
    kryo.writeObject(output1, new TestString("abc"));

    Output output2 = new Output(512, -1);
    kryo = getKryoInstance(true);
    kryo.writeObject(output2, new TestString("abc"));

    Input input1 = new Input(output1.getBuffer());
    kryo = getKryoInstance(false);
    assertEquals("abc", kryo.readObject(input1, TestString.class).str);

    Input input2 = new Input(output2.getBuffer());
    kryo.setReferences(true);
    assertEquals("abc", kryo.readObject(input2, TestString.class).str);
}

static class TestString {
    String str;
    TestString() {
    }
    TestString(String str) {
        this.str = str;
    }
    @Override
    public String toString() {
        return "TestString [str=" + str + "]";
    }
}

private Kryo getKryoInstance(boolean references) {
    Kryo kryo = new Kryo();
    kryo.setRegistrationRequired(false);
    kryo.setReferences(references);
    return kryo;
}

Environment:

  • OS: Windows 10
  • JDK Version: 11
  • Kryo Version: 5.5.0

Additional context
The issue is that CachedFields.newUnsafeField() / CachedFields.newAsmField() check the references flag of the kryo instance and return different CachedField instances depending on that flag. This CachedField is then kept in the FieldSerializer's cachedFields and is not removed if the flag changes, so the wrong CachedField type is used for subsequent deserializations.

@theigl
Copy link
Collaborator

theigl commented May 5, 2023

What is your use-case for changing the reference flag on the fly? Could you work around this limitation by switching between two separate Kryo instances instead of changing the flag?

@thomas-z96
Copy link
Author

Yes, creating a new Kryo instance is the workaround that I used to fix this.
The use-case is that I changed the reference flag to true by default but still need to be able to read older files (that were written with references=false). So when I encounter an old file, I wanted to switch the Kryo instance to reference=false temporarily.

@theigl
Copy link
Collaborator

theigl commented May 8, 2023

Ok thanks for clarifying. Please continue using two separate instances.

It would be rather difficult to reset the cached fields of all field serializers when the flag changes. I think I'll document that the flag cannot be changed on the fly instead of trying to address this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants