vaadin / flow

Vaadin Flow is a Java framework binding Vaadin web components to Java. This is part of Vaadin 10+.
Apache License 2.0
609 stars 167 forks source link

Serialization of scoped beans fails due to the missing UI instance #19967

Open johannest opened 1 week ago

johannest commented 1 week ago

Description of the bug

This issue is tightly connected to kubernetes kit issue: https://github.com/vaadin/kubernetes-kit/issues/140

While discussing with @mcollovati he discovered that as a part of fixing the issue above, there is a need for a fix also in the Flow:

"We need to set the UI threadlocal. Unfortunately we cannot do it in the UI class, because the reference to the bean is held by its parent class Component that is serialized before UI.writeObject() gets called. But we can "fix" this in Flow by adding the thread local logic in Component class. Once this is done, the serialization completes correctly (also needs setting threadlocal in VaadinSession writeObject)"

Expected behavior

Serialization and the de-serialization works also when the application contains scoped beans (@SessionScope, @UIScope, @RouteScope).

Minimal reproducible example

See https://github.com/vaadin/kubernetes-kit/issues/140

The Flow fix could be something along these lines.

com.vaadin.flow.component.Component. Add a Thread Local logic:

@Serial
    private void readObject(ObjectInputStream stream)
            throws IOException, ClassNotFoundException {
        Optional<UI> uiOptional = getUI();
        if (uiOptional.isPresent()) {
            UI ui = uiOptional.get();
            Map<Class<?>, CurrentInstance> old = CurrentInstance.setCurrent(ui);
            try {
                stream.defaultReadObject();
            } finally {
                CurrentInstance.restoreInstances(old);
            }
        }
    }

    @Serial
    private void writeObject(java.io.ObjectOutputStream stream)
            throws IOException {
        Optional<UI> uiOptional = getUI();
        if (uiOptional.isPresent()) {
            UI ui = uiOptional.get();
            Map<Class<?>, CurrentInstance> old = CurrentInstance.setCurrent(ui);
            try {
                CurrentInstance.setCurrent(ui);
                stream.defaultWriteObject();
            } finally {
                CurrentInstance.restoreInstances(old);
            }
        }
    }

VaadinSession improve the existing writeObject with a Thread Local handling:

private void writeObject(java.io.ObjectOutputStream stream)
            throws IOException {
        boolean serializeUIs = true;

        // If service is null it has just been deserialized and should be
        // serialized in
        // the same way again
        if (getService() != null) {
            ApplicationConfiguration appConfiguration = ApplicationConfiguration
                    .get(getService().getContext());
            if (!appConfiguration.isProductionMode() && !appConfiguration
                    .isDevModeSessionSerializationEnabled()) {
                serializeUIs = false;
            }
        }

        Map<Class<?>, CurrentInstance> old = CurrentInstance.setCurrent(this);
            try {
                CurrentInstance.setCurrent(this);
                stream.defaultWriteObject();
            if (serializeUIs) {
                stream.writeObject(uIs);
                stream.writeObject(resourceRegistry);
            } else {
                stream.writeObject(new HashMap<>());
                stream.writeObject(new StreamResourceRegistry(this));
            }
        } finally {
            CurrentInstance.restoreInstances(old);
        }

Versions

mcollovati commented 1 week ago

A few notes:

mcollovati commented 1 week ago

Actually, calling CurrentInstance.setCurrent(ui) when deserializing the UI instance would fail because if calls internals.getSession(), but internals field is not yet set.