As there is no "request end" call after invoking UI.access() from a background thread, the connector map was not earlier properly cleaned afterwards. If you toggled visibility of a component from the background thread, the tracker state became inconsistent. If this becomes a performance problem, it could probably be optimized to that cleanup is done in request end and only at the end of access if not inside a request. Fixes #9654tags/8.1.0
@@ -1418,17 +1418,6 @@ public abstract class VaadinService implements Serializable { | |||
final long duration = (System.nanoTime() - (Long) request | |||
.getAttribute(REQUEST_START_TIME_ATTRIBUTE)) / 1000000; | |||
session.setLastRequestDuration(duration); | |||
// Check that connector tracker is in a consistent state here to | |||
// avoid doing it multiple times for a single request | |||
for (UI ui : session.getUIs()) { | |||
try { | |||
ui.getConnectorTracker().ensureCleanedAndConsistent(); | |||
} catch (AssertionError | Exception e) { | |||
getLogger().log(Level.SEVERE, | |||
"Error cleaning ConnectionTracker", e); | |||
} | |||
} | |||
} finally { | |||
session.unlock(); | |||
} |
@@ -1029,7 +1029,7 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable { | |||
} | |||
try { | |||
ui.getConnectorTracker().cleanConnectorMap(); | |||
} catch (Exception e) { | |||
} catch (AssertionError | Exception e) { | |||
getLogger().log(Level.SEVERE, | |||
"Exception while cleaning connector map for ui " | |||
+ ui.getUIId(), |
@@ -272,17 +272,7 @@ public class ConnectorTracker implements Serializable { | |||
} | |||
cleanStreamVariables(); | |||
} | |||
/** | |||
* Performs expensive checks to ensure that the connector tracker is cleaned | |||
* properly and in a consistent state. | |||
* <p> | |||
* This should only be called by the framework. | |||
* | |||
* @since 8.1 | |||
*/ | |||
public void ensureCleanedAndConsistent() { | |||
// Do this expensive check only with assertions enabled | |||
assert isHierarchyComplete() : "The connector hierarchy is corrupted. " | |||
+ "Check for missing calls to super.setParent(), super.attach() and super.detach() " | |||
@@ -315,6 +305,10 @@ public class ConnectorTracker implements Serializable { | |||
} else if (!uninitializedConnectors.contains(connector) | |||
&& !LegacyCommunicationManager | |||
.isConnectorVisibleToClient(connector)) { | |||
// Connector was visible to the client but is no longer (e.g. | |||
// setVisible(false) has been called or SelectiveRenderer tells | |||
// it's no longer shown) -> make sure that the full state is | |||
// sent again when/if made visible | |||
uninitializedConnectors.add(connector); | |||
diffStates.remove(connector); | |||
assert isRemovalSentToClient(connector) : "Connector " |
@@ -23,7 +23,7 @@ import java.util.logging.LogRecord; | |||
import java.util.logging.Logger; | |||
import com.vaadin.server.VaadinRequest; | |||
import com.vaadin.server.VaadinService; | |||
import com.vaadin.server.VaadinSession; | |||
import com.vaadin.tests.components.AbstractReindeerTestUIWithLog; | |||
import com.vaadin.ui.Button; | |||
import com.vaadin.ui.Button.ClickEvent; | |||
@@ -60,15 +60,15 @@ public class MissingHierarchyDetection extends AbstractReindeerTestUIWithLog { | |||
@Override | |||
protected void setup(VaadinRequest request) { | |||
// Catch log messages so we can see if there is an error | |||
Logger vaadinServiceLogger = Logger | |||
.getLogger(VaadinService.class.getName()); | |||
vaadinServiceLogger.addHandler(new Handler() { | |||
Logger vaadinSessionLogger = Logger | |||
.getLogger(VaadinSession.class.getName()); | |||
vaadinSessionLogger.addHandler(new Handler() { | |||
@Override | |||
public void publish(LogRecord record) { | |||
if (record.getThrown() instanceof AssertionError) { | |||
pendingErrors.add(record); | |||
vaadinServiceLogger.removeHandler(this); | |||
vaadinSessionLogger.removeHandler(this); | |||
} | |||
} | |||
@@ -0,0 +1,62 @@ | |||
/* | |||
* Copyright 2000-2016 Vaadin Ltd. | |||
* | |||
* Licensed under the Apache License, Version 2.0 (the "License"); you may not | |||
* use this file except in compliance with the License. You may obtain a copy of | |||
* the License at | |||
* | |||
* http://www.apache.org/licenses/LICENSE-2.0 | |||
* | |||
* Unless required by applicable law or agreed to in writing, software | |||
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT | |||
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the | |||
* License for the specific language governing permissions and limitations under | |||
* the License. | |||
*/ | |||
package com.vaadin.tests.push; | |||
import com.vaadin.annotations.Push; | |||
import com.vaadin.annotations.Widgetset; | |||
import com.vaadin.server.VaadinRequest; | |||
import com.vaadin.ui.Button; | |||
import com.vaadin.ui.Label; | |||
import com.vaadin.ui.UI; | |||
import com.vaadin.ui.VerticalLayout; | |||
@Push | |||
@Widgetset("com.vaadin.DefaultWidgetSet") | |||
public class PushToggleComponentVisibility extends UI { | |||
@Override | |||
protected void init(VaadinRequest request) { | |||
VerticalLayout mainLayout = new VerticalLayout(); | |||
setContent(mainLayout); | |||
Label label = new Label("Please wait"); | |||
label.setId("label"); | |||
label.setVisible(false); | |||
mainLayout.addComponent(label); | |||
Button button = new Button("Hide me 3 secondes"); | |||
button.setId("hide"); | |||
button.addClickListener(event1 -> { | |||
button.setVisible(false); | |||
label.setVisible(true); | |||
new Thread(() -> { | |||
try { | |||
Thread.sleep(3000); | |||
} catch (InterruptedException e) { | |||
e.printStackTrace(); | |||
} | |||
button.getUI().access(() -> { | |||
button.setVisible(true); | |||
label.setVisible(false); | |||
button.getUI().push(); | |||
}); | |||
}).start(); | |||
}); | |||
mainLayout.addComponent(button); | |||
} | |||
} |
@@ -0,0 +1,43 @@ | |||
/* | |||
* Copyright 2000-2016 Vaadin Ltd. | |||
* | |||
* Licensed under the Apache License, Version 2.0 (the "License"); you may not | |||
* use this file except in compliance with the License. You may obtain a copy of | |||
* the License at | |||
* | |||
* http://www.apache.org/licenses/LICENSE-2.0 | |||
* | |||
* Unless required by applicable law or agreed to in writing, software | |||
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT | |||
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the | |||
* License for the specific language governing permissions and limitations under | |||
* the License. | |||
*/ | |||
package com.vaadin.tests.push; | |||
import org.junit.Assert; | |||
import org.junit.Test; | |||
import org.openqa.selenium.By; | |||
import com.vaadin.testbench.elements.ButtonElement; | |||
import com.vaadin.testbench.elements.LabelElement; | |||
import com.vaadin.tests.tb3.SingleBrowserTest; | |||
public class PushToggleComponentVisibilityTest extends SingleBrowserTest { | |||
private static final String HIDE = "hide"; | |||
@Test | |||
public void ensureComponentVisible() { | |||
openTestURL(); | |||
$(ButtonElement.class).id(HIDE).click(); | |||
Assert.assertEquals("Please wait", | |||
$(LabelElement.class).first().getText()); | |||
waitForElementPresent(By.id(HIDE)); | |||
$(ButtonElement.class).id(HIDE).click(); | |||
Assert.assertEquals("Please wait", | |||
$(LabelElement.class).first().getText()); | |||
} | |||
} |