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

[NETBEANS-4054] Ensuring progress when javac crashes while batch evaluating hints. #5384

Merged
merged 1 commit into from
Jan 30, 2023

Conversation

jlahoda
Copy link
Contributor

@jlahoda jlahoda commented Jan 29, 2023

If hints are run on save, BatchSearch is called on a single file. Normally, if javac would crash during toPhase, an exception would be thrown. But, it may happen the existing parser is reused, and toPhase does not throw the exception, and this may be mistaken for an out-of-memory situation, to which the response is to restart the processing. This may then lead to an infinite loop.

The proposed change here is to force progress in this situation. javac crash(es) need to be handled separately, of course.


^Add meaningful description above

By opening a pull request you confirm that, unless explicitly stated otherwise, the changes -

  • are all your own work, and you have the right to contribute them.
  • are contributed solely under the terms and conditions of the Apache License 2.0 (see section 5 of the license for more information).

Please make sure (eg. git log) that all commits have a valid name and email address for you in the Author field.

If you're a first time contributor, see the Contributing guidelines for more information.

If you're a committer, please label the PR before pressing "Create pull request" so that the right test jobs can run.

@jlahoda jlahoda added this to the NB17 milestone Jan 29, 2023
@jlahoda jlahoda requested a review from mbien January 29, 2023 14:12
@neilcsmith-net neilcsmith-net added the Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) label Jan 29, 2023
@apache apache locked and limited conversation to collaborators Jan 29, 2023
@apache apache unlocked this conversation Jan 29, 2023
@mbien mbien added the hints label Jan 29, 2023
Copy link
Member

@mbien mbien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me, this is actually somewhat similar to the hotfix I proposed here it just limits it to single-file use only.

Comment on lines -249 to +258
if (parameter.toPhase(Phase.RESOLVED).compareTo(Phase.RESOLVED) < 0)
if (parameter.toPhase(Phase.RESOLVED).compareTo(Phase.RESOLVED) < 0) {
if (currentInputList.size() == 1) {
//the javac crashed while processing the (single) file, we must ensure progress, otherwise infinite loop in processing would happen:
problems.add(new MessageImpl(MessageKind.WARNING, "An error occurred while processing file: " + FileUtil.getFileDisplayName(parameter.getFileObject()) + ", please see the IDE log for more information."));
currentPointer.incrementAndGet();
}

return ;
}
Copy link
Member

@mbien mbien Jan 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this means that this can't happen when BatchSearch is used on multiple files because it wouldn't reuse javac instances there?

So the only reason it tries again with the same file when Phase.RESOLVED can't be reached is because of a possible OOM situation?

If that is the case we should add some kind of per-file retry counter. while(true) loops are always a bit of a danger zone.

Copy link
Member

@mbien mbien Jan 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry never mind this, I just noticed @jlahoda posted a long explanation as comment on the issue which answers the questions. Although the retry counter might be still something we could consider to ensure there is no scenario where this loop can't exit.

@neilcsmith-net neilcsmith-net merged commit 3e4dd33 into apache:delivery Jan 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hints Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Netbeans freezes due to "Organize Imports" on save and "Format" Tool if code contains specific syntax errors
3 participants