Repository navigation
Bugfix for #2453: NPE in BarContentAssistProcessor. - #2484
nettozahler wants to merge 1 commit into
Conversation
|
This pull request changes some projects for the first time in this development cycle. An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch. Git patchFurther information are available in Common Build Issues - Missing version increments. |
|
While null-check are always welcome, I'm more curious about in which context can this issue be happening? The document isn't supposed to be null at that stage, so maybe the issue is earlier in the execution chain? |
|
Since the affected class belongs to test code and the NPE happens when locally running |
|
To answer the question above about how the document can be null here: the editor is closed in the test teardown while This has been harder to hit since August 2025, when a null check was introduced in f70ee0a. A small window remains, because the document is fetched again after the check without a guard. So the NPE is suppressed in practice, but the underlying race is still there (if you consider it a problem in test code). To reproduce
try {
Thread.sleep(300); // temporary: let teardown close the editor first
} catch (InterruptedException e) {
// ignore
}
Result: Root cause
The sleep makes the processor outlast the interrupt and the editor close (1 and 2); removing the null check supplies 3. Suggested fix Read the document once into a local variable, return no proposals if it is null, and use that variable throughout the method. |
|
Thanks for the detailed problem analysis. IMHO my fix in this PR contains exactly what you have suggested. But currently this old fix-branch has conflicts that must be resolved before merging. |
|
Yes, your change covers what I suggested. Would you like to rebase the branch to resolve the conflicts? If you’d rather not, I can open a new PR on current master with your change, crediting you as co-author. |
|
I can do the rebase when I find some time. If you have some pressure and do not want to wait, you can also open a new PR as suggested. |
|
Ok, thank you - please go ahead, no pressure from my side. Just wanted to follow up. |
113a645 to
ca6de48
Compare
|
I rebased and force-pushed the PR. Now we need someone who is willing to merge it. Maybe @akurtakov ? |
Fixes #2453 .
This bugfix simply adds a check for NULL which was missing. In case of NULL a default value is returned.