netbeans
netbeans copied to clipboard
apache
Netbeans freezes due to "Organize Imports" on save and "Format" Tool if code contains specific syntax errors
Apache NetBeans version
Apache NetBeans 13, 15
What happened
The "Format" Tool and "Organize Import" seem crash netbeans sometimes. This might be related to the code containg specific syntax errors, because I can reproduce the bug when inserting C++ code. This bug happend to me before but not as often in Netbeans 13 as in 12.5
How to reproduce
This bug happens every once in a while to me, but I found a way to reproduce it for me:
I ported the code from https://github.com/Gregjksmith/Iterative-Closest-Point/blob/master/src/KdTree.cpp for a project to java and tried to save this file https://pastebin.com/NXWnF1Hu with "Organize Import" as on save action enabled. Netbeans did not respond anymore and did not save the file. This specific code did not crash trying to use "Format" but others in th past did.
Did this work correctly in an earlier version?
No
Operating System
Windows 10.0
JDK
Oracle JDK 17 (17+35-LTS-2724)
Apache NetBeans packaging
Apache NetBeans provided installer
Anything else
No response
Are you willing to submit a pull request?
No
Code of Conduct
Yes
Hello, facing the same issue since a while. Its rely annoying. Seems the same reported here with logs: #4515
can be reproduced with a short snippet (taken from the pastebin file of this issue):
package main;
public class Test {
void gs::KdTree::build(std::vector<Point*> &pointCloud, int start, int end, int sortOn) {
if (end - start == 1) {
__node = new Point(pointCloud[start]->pos[0], pointCloud[start]->pos[1], pointCloud[start]->pos[2]);
return;
}
}
}
paste this into a java file and save while having "Organize Imports" on save enabled.
it will then enter an endless while-loop in (set breakpoint to that line to safely reproduce):
at org.netbeans.modules.java.hints.spiimpl.batch.BatchSearch.getLocalVerifiedSpans(BatchSearch.java:233)
at org.netbeans.modules.java.hints.spiimpl.batch.BatchSearch.access$400(BatchSearch.java:76)
at org.netbeans.modules.java.hints.spiimpl.batch.BatchSearch$LocalIndexEnquirer.validateResource(BatchSearch.java:550)
at org.netbeans.modules.java.hints.spiimpl.batch.BatchSearch.getVerifiedSpans(BatchSearch.java:183)
at org.netbeans.modules.java.hints.spiimpl.batch.BatchUtilities.applyFixes(BatchUtilities.java:174)
at org.netbeans.modules.java.hints.onsave.RemoveUnusedAfterSave.performTask(RemoveUnusedAfterSave.java:85)
at org.netbeans.modules.editor.lib.BeforeSaveTasks$TaskRunnable$1.run(BeforeSaveTasks.java:148)
at org.netbeans.editor.GuardedDocument.runAtomicAsUser(GuardedDocument.java:333)
at org.netbeans.modules.editor.lib.BeforeSaveTasks$TaskRunnable.run(BeforeSaveTasks.java:131)
at org.netbeans.modules.editor.lib.TrailingWhitespaceRemove.runLocked(TrailingWhitespaceRemove.java:77)
at org.netbeans.modules.editor.lib.BeforeSaveTasks$TaskRunnable.run(BeforeSaveTasks.java:128)
at org.netbeans.modules.editor.impl.ReformatBeforeSaveTask.runLocked(ReformatBeforeSaveTask.java:101)
at org.netbeans.modules.editor.lib.BeforeSaveTasks$TaskRunnable.run(BeforeSaveTasks.java:128)
at org.netbeans.modules.java.hints.onsave.RemoveUnusedAfterSave$1.run(RemoveUnusedAfterSave.java:98)
at org.netbeans.modules.java.hints.onsave.RemoveUnusedAfterSave$1.run(RemoveUnusedAfterSave.java:96)
at org.netbeans.api.java.source.JavaSource$MultiTask.run(JavaSource.java:504)
at org.netbeans.modules.parsing.impl.TaskProcessor.callUserTask(TaskProcessor.java:586)
at org.netbeans.modules.parsing.api.ParserManager$UserTaskAction.run(ParserManager.java:132)
at org.netbeans.modules.parsing.api.ParserManager$UserTaskAction.run(ParserManager.java:116)
at org.netbeans.modules.parsing.impl.TaskProcessor$2.call(TaskProcessor.java:181)
at org.netbeans.modules.parsing.impl.TaskProcessor$2.call(TaskProcessor.java:178)
at org.netbeans.modules.masterfs.filebasedfs.utils.FileChangedManager.priorityIO(FileChangedManager.java:153)
at org.netbeans.modules.masterfs.providers.ProvidedExtensions.priorityIO(ProvidedExtensions.java:335)
at org.netbeans.modules.parsing.nb.DataObjectEnvFactory.runPriorityIO(DataObjectEnvFactory.java:118)
at org.netbeans.modules.parsing.impl.Utilities.runPriorityIO(Utilities.java:67)
at org.netbeans.modules.parsing.impl.TaskProcessor.runUserTask(TaskProcessor.java:178)
at org.netbeans.modules.parsing.api.ParserManager.parse(ParserManager.java:83)
at org.netbeans.api.java.source.JavaSource.runUserActionTaskImpl(JavaSource.java:454)
at org.netbeans.api.java.source.JavaSource.runUserActionTask(JavaSource.java:425)
at org.netbeans.modules.java.hints.onsave.RemoveUnusedAfterSave.runLocked(RemoveUnusedAfterSave.java:96)
at org.netbeans.modules.editor.lib.BeforeSaveTasks$TaskRunnable.run(BeforeSaveTasks.java:128)
at org.netbeans.modules.editor.lib.BeforeSaveTasks.runTasks(BeforeSaveTasks.java:105)
at org.netbeans.modules.editor.lib.BeforeSaveTasks$2.run(BeforeSaveTasks.java:86)
... more....
it loops between while and the phase check in the task since nothing decrements toProcess and the controller never reaches RESOLVED phase:
while (currentPointer.get() < toProcess.size()) {
if (cancel.get())
return;
final AtomicBoolean stop = new AtomicBoolean();
// JavaSource js = JavaSource.create(e.getKey(), f);
JavaSource js = JavaSource.create(e.getKey(), toProcess.subList(currentPointer.get(), toProcess.size()));
js.runUserActionTask(new Task<CompilationController>() {
@Override
public void run(CompilationController parameter) throws Exception {
if (stop.get()) return;
if (cancel.get()) return;
boolean cont = true;
try {
if (parameter.toPhase(Phase.RESOLVED).compareTo(Phase.RESOLVED) < 0)
return ;
@jlahoda could you take a look at this? A reproducer is in the comment above this one.
Should all return statements which leave the task, also increment the currentPointer counter?
something this would break the infinite loop:
diff --git a/java/spi.java.hints/src/org/netbeans/modules/java/hints/spiimpl/batch/BatchSearch.java b/java/spi.java.hints/src/org/netbeans/modules/java/hints/spiimpl/batch/BatchSearch.java
index f01480d..d2ce275 100644
--- a/java/spi.java.hints/src/org/netbeans/modules/java/hints/spiimpl/batch/BatchSearch.java
+++ b/java/spi.java.hints/src/org/netbeans/modules/java/hints/spiimpl/batch/BatchSearch.java
@@ -246,8 +246,10 @@
boolean cont = true;
try {
- if (parameter.toPhase(Phase.RESOLVED).compareTo(Phase.RESOLVED) < 0)
+ if (parameter.toPhase(Phase.RESOLVED).compareTo(Phase.RESOLVED) < 0) {
+ currentPointer.incrementAndGet();
return ;
+ }
progress.setMessage("processing: " + FileUtil.getFileDisplayName(parameter.getFileObject()));
Resource r = file2Resource.get(parameter.getFileObject());
Hm - I don't think incrementing the pointer when toPhase does not reach the expected phase is correct - that would might mean the given file won't be run again. We may need to add some trick to detect an exception from javac and skip the file - here, and also in JavacParser. Trying to look into that. (Not to speak about the need to actually fix the exception.)
@mbien @jlahoda any further thoughts on this? It's marked high for NB17, so a potential reason to delay the release.
@neilcsmith-net I don't think we should delay because of this since it is not a regression. It is already there since several releases. Yet its high priority on the things we should fix :)
@mbien thanks. That's really a discussion for last RC if we haven't integrated a fix. Just waking up the discussion. High priority is a potential reason to delay, critical is a definite reason to delay, same as always. I think the priority is probably right on this - IDE shouldn't be locking up.
the main reason why I didn't put the critical label on it is because it isn't enabled by default. But I fully agree that we should fix it.
I probably won't be able to fix the root cause of this since I don't have enough javac experience, however, I could try to make the code behave a bit more defensively, so that even if something breaks it would give up instead of locking up.
Hm - I don't think incrementing the pointer when toPhase does not reach the expected phase is correct - that would might mean the given file won't be run again.
I don't understand this part tbh. Why would anything change if it is ran again with the exact same state? Also when stop is set to true it will be reset on the next loop iteration without ever doing anything?
the original code when imported the first time looked like this: https://github.com/emilianbold/netbeans-releases/blob/2216f9361692d8bf818c086e028e86aff0ffaf7f/java.hints/src/org/netbeans/modules/java/hints/jackpot/impl/batch/BatchSearch.java#L297-L323
comment links to this bug: https://bz.apache.org/netbeans/show_bug.cgi?id=192481
Hm - I don't think incrementing the pointer when toPhase does not reach the expected phase is correct - that would might mean the given file won't be run again.
I don't understand this part tbh. Why would anything change if it is ran again with the exact same state? Also when stop is set to true it will be reset on the next loop iteration without ever doing anything?
So, NetBeans, for a long time, allowed to process projects that would not fit into the memory while parsed&attributed. There were several tricks combined to let that happen, and one of them ability to re-load a class, that is already load from a classfile, from a source. That was part of the patched nb-javac, as ordinary javac cannot do that.
So, while processing many source files, it created an instance of javac, invoked the task for the first file, the task typically parsed&attributed (using all dependencies from classfiles), then the task was invoked for the second file (possibly re-loading an existing class load from classfile, but this time from source), etc. And if, at some point, the javac instance could not fit into memory, the toPhase signaled that, the task returned immediately, the infrastructure thrown away that instance of javac, created a new one, and started the task with the given file again (which then parsed&attributed it in the new javac instance, etc.). So, multi-file tasks generally can be called multiple (2) times for the same file, if there's not enough memory to process all at once.
With the removal of nb-javac, we've lost the ability to re-load classes from sources, so there was a need to do it differently. So, the current approach is "all at once or file-by-file" (either all files will fit into the memory, or the processing will run slowly creating a javac instance for each file), but it may still happen the processing will start in the "all" way, and then drop to "file-by-file", and that is again indicated by the toPhase result. I.e. if the achieved phase is not high enough, the task should stop, and it will be called again (in a file-by-file mode in practice, but the task should not care). So, this is the reason why we can't blindly increment the pointer, as that may lead to skipping some files (although in practice it would be rare, see below).
The reason for the pointer is that hint may in general force a change in dependencies - they may, e.g. say that a new dependency should be added, and we need to throw away the javac instance, and create a new one. This is what the pointer and stop variable are trying to achieve. In practice, this is not really common, and the while (currentPointer.get() < toProcess.size()) loop is executed only once.
Also, normally when javac crashes during toPhase, there will actually be an exception thrown, which is handled properly by the task.
In this particular case, however, there is a single file, so most of the machinery described above is not useful, and it is also a file opened in the editor, and hence javac has already been run on it, so not exception is thrown.
I was doing some experiments, and the solution probably is incrementing the pointer, we just need to limit that to only these conditions. I should be able to submit a PR shortly.
the original code when imported the first time looked like this: https://github.com/emilianbold/netbeans-releases/blob/2216f9361692d8bf818c086e028e86aff0ffaf7f/java.hints/src/org/netbeans/modules/java/hints/jackpot/impl/batch/BatchSearch.java#L297-L323
comment links to this bug: https://bz.apache.org/netbeans/show_bug.cgi?id=192481
@jlahoda thanks a lot for the detailed explanation. This explains why I couldn't figure out why NB would want to retry the same file when it can't be Resolved on the first attempt.
Many thanks for the fix and detailed explanation @jlahoda
PR is merged to delivery and will be in 17-rc3. Closing this issue, but please do some checking when the release candidate is announced in a few days.