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

Error "Trying to start a new request while another is active" in browser while setting the theme, pushing, and adding a grid in the same request. #11954

Closed
eriklumme opened this issue Apr 22, 2020 · 4 comments · Fixed by #12043

Comments

@eriklumme
Copy link
Contributor

eriklumme commented Apr 22, 2020

Steps to reproduce

  1. Enable @Push
  2. In a click listener, do the following
    2.1. Set the theme to an existing theme other than the current using UI#setTheme
    2.2. Manually push using UI#push
    2.3. Add a Grid

Actual outcome

An error Trying to start a new request while another is active is displayed in the browser console.

I believe this is the root cause of sometimes infinite resynchronizations of the page in a real application, but I have not been able to reproduce it completely. The real app does not use manual pushing, but it does have push enabled (long polling) and it accesses the UI from background threads.

Expected outcome

No errors in the console.

Example code

After clicking the first button, the theme must be reset before the error occurs again.

@Theme("mytheme")
@Push
public class MyUI extends UI {

    @Override
    protected void init(VaadinRequest vaadinRequest) {
        VerticalLayout layout = new VerticalLayout();
        UI ui = UI.getCurrent();

        Button button = new Button("Explosion button");
        button.addClickListener(e -> {
            // Must be a different theme, and its style files must exist
            ui.setTheme("my-theme");
            ui.push();
            layout.addComponent(new Grid<>());
        });

        Button reset = new Button("Reset theme");
        reset.addClickListener(e -> ui.setTheme("mytheme"));
        
        layout.addComponents(button, reset);
        setContent(layout);
    }
    
    @WebServlet(urlPatterns = "/*", name = "MyUIServlet", asyncSupported = true)
    @VaadinServletConfiguration(ui = MyUI.class, productionMode = false)
    public static class MyUIServlet extends VaadinServlet {
    }
}
@eriklumme eriklumme changed the title Error "Trying to start a new request while another is active" in browser while setting the theme, pushing, and adding a component with a data provider in the same request. Error "Trying to start a new request while another is active" in browser while setting the theme, pushing, and adding a grid in the same request. Apr 22, 2020
@eriklumme
Copy link
Contributor Author

I initially thought it was reproducible with a ComboBox too, but it seems that was just due to my session being serialized across server restarts.

@ShawnRG
Copy link

ShawnRG commented May 19, 2020

What version of vaadin are you using?

@ShawnRG
Copy link

ShawnRG commented May 19, 2020

If you're using background threads to update your UI you should always provide them with the UI instance as since vaadin version 8 the UI.getCurrent() is a weak reference to avoid memory leaks.
So you you could cache your UI in either of the following ways:

// This should be called from the main UI thread.
final UI ui = UI.getCurrent();

or preferably when calling from within a Component

// From within the component
final UI ui = getUI();

Then what you should do is wrap your code used in the ClickListener in a UI.access() runnable block, like so:

final UI ui = UI.getCurrent();
ui.access(() -> {
    ui.setTheme("mytheme2");
    ui.push();
    layout.addComponent(new Grid<>());
})

Hope this helps

@eriklumme
Copy link
Contributor Author

eriklumme commented May 20, 2020

Hi @ShawnRG . Thanks for your reply, but as you can see from the MWE it's not an issue with background threads. It should be reproducible with any Vaadin 8.X version.

It seems to be an issue with Vaadin's MessageSender not taking active requests into account when issuing a resynchronization due to theme changes.

I have made a PoC for a solution here
8.8...eriklumme:8.8

Ansku added a commit to Ansku/framework that referenced this issue Jun 29, 2020
There might be pending requests in the queue when a resync request is
made (e.g. through a theme change). This can cause conflicts if the
resync request is handled immediately. Therefore the resync request
should also be added to the queue and only get resolved when
doSendInvocationsToServer() gets triggered again. Consequently we can
also avoid unnecessary replacing of elements that simply have their
theme changed while still updating elements that display theme resources
where the resource url changes with the theme change.

Fixes vaadin#11954
Ansku added a commit to Ansku/framework that referenced this issue Jul 23, 2020
There might be pending requests in the queue when a resync request is
made (e.g. through a theme change). This can cause conflicts if the
resync request is handled immediately. Therefore the resync request
should also be added to the queue and only get resolved when
doSendInvocationsToServer() gets triggered again. Consequently we can
also avoid unnecessary replacing of elements that simply have their
theme changed while still updating elements that display theme resources
where the resource url changes with the theme change.

Fixes vaadin#11954
Ansku added a commit to Ansku/framework that referenced this issue Jul 30, 2020
There might be pending requests in the queue when a resync request is
made (e.g. through a theme change). This can cause conflicts if the
resync request is handled immediately. Therefore the resync request
should also be added to the queue and only get resolved when
doSendInvocationsToServer() gets triggered again.

Fixes vaadin#11954
Ansku added a commit that referenced this issue Jul 30, 2020
There might be pending requests in the queue when a resync request is
made (e.g. through a theme change). This can cause conflicts if the
resync request is handled immediately. Therefore the resync request
should also be added to the queue and only get resolved when
doSendInvocationsToServer() gets triggered again.

Fixes #11954
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants