NullPointerException in StateNode when setting locale and using UI.getCurrent() inside localeChange callback. #11599
Comments
mrgreywater
changed the title
|
This is typically the symptom of doing changes without properly locking the session. How is that locale change applied? Vaadin will apply some additional checks for potentially missing the lock if the JVM is run with assertions enabled ( |
|
Adding the -ea parameter doesn't seem to trigger any assertion. I get the following message as well (but only after this exception is thrown a few times): and
Here is how the locale is changed: https://github.com/KasparScherrer/language-select/blob/master/src/main/java/ch/carnet/kasparscherrer/LanguageSelect.java#L68 It seems to only happen when the same application is open in multiple tabs. And even then you have to change locale multiple times, reload pages until at some point the exception triggers and breaks the page. Once the exception triggers once, the tab where it triggers stays in a broken state, after that even creating new tabs trigger the exception immediately. Only when reloading with a new session (other browser, new incognito window) it's fixed. Please also note that in the whole exception call stack, it doesn't reach my own code at any point. Vaadin just gets stuck in some kind of broken state. |
|
Thanks for the additional details. There doesn't seem to be any direct indication of missing session locking in this case since the locale change is triggered from the value change listener of a Vaadin component. There could in theory be some hidden causes e.g. if some attached component does something unconventional in their That leaves the possibility that there's something broken with the state management related to |
|
I don't know this internal code at all, but I find it curious that both the function Btw, here are the |
|
Can't say whether the workaround for this even exists, before making a minimal reproduce example. |
|
I couldn't reproduce this exception.
My OS is Mac, not Windows, but not sure this impacts the result anyhow. |
|
I can't reproduce it with your test project either, but in my original project I can. I'll try to figure out out what's missing to reproduce the issue (could be the I18NProvider, a side effect of running multiple SpringApplets, or side effect of something inside the localeChange callbacks). |
|
Here some notes on the changes I did to the my-app project to reproduce the bug: Replaced the function HelloWorldView public HelloWorldView() {
addClassName("hello-world-view");
LanguageSelect languageSelect = new LanguageSelect(Locale.CANADA, Locale.CHINA, Locale.FRANCE);
Grid<String> grid = new Grid<>();
Grid.Column<String> column = grid.addColumn(o -> "");
column.setHeader(new LocalizedDatePicker());
add(languageSelect, grid);
}added package com.example.application.components;
import com.vaadin.flow.component.UI;
import com.vaadin.flow.component.datepicker.DatePicker;
import com.vaadin.flow.i18n.LocaleChangeEvent;
import com.vaadin.flow.i18n.LocaleChangeObserver;
import java.time.LocalDate;
import java.util.Locale;
public class LocalizedDatePicker extends DatePicker implements LocaleChangeObserver {
public LocalizedDatePicker() {
}
private void replaceJSDateParsing(Locale locale) {
// this check has been added after the fact
if (getUI().orElse(null) != UI.getCurrent()) {
System.err.println("UI mismatch!");
}
UI.getCurrent().beforeClientResponse(this, ctx ->
// in the original function, actual useful code is called instead
getElement().executeJs("console.log(this)")
);
}
@Override
public void setLocale(Locale locale) {
super.setLocale(locale);
this.replaceJSDateParsing(locale);
}
@Override
public void localeChange(LocaleChangeEvent event) {
setLocale(event.getLocale());
}
}It seems the issue stems from the fact that in the localeChange callback UI.getCurrent() returns an mismatching UI object sometimes. I'm not quite sure if this is as intended, or if the thread local UI.getCurrent() UI object should match the Component::getUI object inside the callbacks (This is what I assumed). This was quite difficult to figure out, because the exception from Vaadin gives no clue on where the problem lies. I would love if the exceptions could be thrown earlier in cases like this with more comprehensible messages (Perhaps one could add an assertion in in |
mrgreywater
changed the title
|
Thanks for the detailed analysis. From this, I can make an educated guess that we have two different bugs on the radar:
There might also be something related to having push enabled, even though it could also be that it only contributes to making the state corruption visible without having any issues of its own. |
|
Thanks, @mrgreywater , now with your codes it repeats every run. @Legioth , concerning point 1), why do we need to update |
|
Instead, the iteration should be something along these lines: |
Fixes #11599 Co-authored-by: Mikhail Shabarov <[email protected]>
Fixes #11599 Co-authored-by: Mikhail Shabarov <[email protected]>
…#11788) Related-to #11599 Co-authored-by: Mikhail Shabarov <[email protected]>
|
This ticket/PR has been released with platform 21.0.1. |
|
This ticket/PR has been released with platform 14.7.1. |
|
This ticket/PR has been released with platform 20.0.8. |
Description of the bug / feature
NullPointerException when changing locale
Minimal reproducible example
Sometimes when changing the locale when Push is enabled, Vaadin throws a NullPointerException. It's difficult to reproduce.
Expected behavior
No Exception should be thrown
Actual behavior
A NullPointer Exception is thrown because in StateNode::dumpBeforeClientResponseEntries() there is a check missing for
entries == nullVersions:
The text was updated successfully, but these errors were encountered: