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

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

Closed
johannest opened this issue Sep 17, 2024 · 2 comments · Fixed by #20394
Closed

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

johannest opened this issue Sep 17, 2024 · 2 comments · Fixed by #20394

Comments

@johannest
Copy link
Contributor

Description of the bug

This issue is tightly connected to kubernetes kit issue: vaadin/kubernetes-kit#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 vaadin/kubernetes-kit#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

  • Vaadin / Flow version: 24.4.11
  • Java version: 17
  • OS version: Mac (M2), Kubernetes Kit 2.2.2
@mcollovati
Copy link
Collaborator

mcollovati commented Sep 17, 2024

A few notes:

  • Component.readObject might not be needed
  • We need to set the thread local only when serializing UI object, so the writeMethod could be
      private void writeObject(java.io.ObjectOutputStream stream)
              throws IOException {
          if (this instanceof UI ui) {
              Map<Class<?>, CurrentInstance> old = CurrentInstance.setCurrent(ui);
              try {
                  stream.defaultWriteObject();
              } finally {
                  CurrentInstance.restoreInstances(old);
              }
          } else {
              stream.defaultWriteObject();
          }
      }
    

@mcollovati
Copy link
Collaborator

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

johannest added a commit to johannest/flow that referenced this issue Sep 18, 2024
ThreadLocal handling for VaadinSession and Component
@mcollovati mcollovati self-assigned this Oct 30, 2024
mcollovati added a commit that referenced this issue Nov 4, 2024
Makes sure that VaadinSession and UI thread locals are available during
both serialization and deserialization, to allow other libraries to
perform inspection and injection of Vaadin scoped beans.

Also refactors VaadinRouteScope to be independent from VaadinService
when fetching RouteScopeOwner annotation for the bean, and replaces
VaadinSession.unlock() calls with direct access to the lock instance
to prevent unwanted push during bean lookup.

Fixes #19967
Part of vaadin/kubernetes-kit#140
mcollovati added a commit that referenced this issue Nov 4, 2024
Makes sure that VaadinSession and UI thread locals are available during
both serialization and deserialization, to allow other libraries to
perform inspection and injection of Vaadin scoped beans.

Also refactors VaadinRouteScope to be independent from VaadinService
when fetching RouteScopeOwner annotation for the bean, and replaces
VaadinSession.unlock() calls with direct access to the lock instance
to prevent unwanted push during bean lookup.

Fixes #19967
Part of vaadin/kubernetes-kit#140
mshabarov added a commit that referenced this issue Nov 7, 2024
Makes sure that VaadinSession and UI thread locals are available during
both serialization and deserialization, to allow other libraries to
perform inspection and injection of Vaadin scoped beans.

Also refactors VaadinRouteScope to be independent from VaadinService
when fetching RouteScopeOwner annotation for the bean, and replaces
VaadinSession.unlock() calls with direct access to the lock instance
to prevent unwanted push during bean lookup.

Fixes #19967
Part of vaadin/kubernetes-kit#140

Co-authored-by: Mikhail Shabarov <[email protected]>
vaadin-bot pushed a commit that referenced this issue Nov 7, 2024
Makes sure that VaadinSession and UI thread locals are available during
both serialization and deserialization, to allow other libraries to
perform inspection and injection of Vaadin scoped beans.

Also refactors VaadinRouteScope to be independent from VaadinService
when fetching RouteScopeOwner annotation for the bean, and replaces
VaadinSession.unlock() calls with direct access to the lock instance
to prevent unwanted push during bean lookup.

Fixes #19967
Part of vaadin/kubernetes-kit#140

Co-authored-by: Mikhail Shabarov <[email protected]>
mcollovati added a commit that referenced this issue Nov 7, 2024
Makes sure that VaadinSession and UI thread locals are available during
both serialization and deserialization, to allow other libraries to
perform inspection and injection of Vaadin scoped beans.

Also refactors VaadinRouteScope to be independent from VaadinService
when fetching RouteScopeOwner annotation for the bean, and replaces
VaadinSession.unlock() calls with direct access to the lock instance
to prevent unwanted push during bean lookup.

Fixes #19967
Part of vaadin/kubernetes-kit#140

Co-authored-by: Marco Collovati <[email protected]>
Co-authored-by: Mikhail Shabarov <[email protected]>
mcollovati added a commit that referenced this issue Nov 7, 2024
Makes sure that VaadinSession and UI thread locals are available during
both serialization and deserialization, to allow other libraries to
perform inspection and injection of Vaadin scoped beans.

Also refactors VaadinRouteScope to be independent from VaadinService
when fetching RouteScopeOwner annotation for the bean, and replaces
VaadinSession.unlock() calls with direct access to the lock instance
to prevent unwanted push during bean lookup.

Fixes #19967
Part of vaadin/kubernetes-kit#140

Co-authored-by: Mikhail Shabarov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging a pull request may close this issue.

3 participants