Community
Participate
Working Groups
As detailed in bug 418092, if a completion proposer throws a runtime exception, the entire Eclipse UI can be held hostage. Protection against poorly behaving completion proposers needs to be provided. Before fixing bug 418092, I experimented with wrapping the call to handleSetData() in CompletionProposalPopup.java to unregister the popup before rethrowing the runtime exception. The following git patch did the trick. For the Guava Sets class, ArrayOutOfBoundExceptions were shown in the error log for the method signatures that contained parameterized annotations, but other method signatures were shown in the completions popup and the UI did not get locked up. diff --git a/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/CompletionProposalPopup.java b/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/CompletionProposalPopup.java index b748eab..b8134d8 100644 --- a/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/CompletionProposalPopup.java +++ b/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/CompletionProposalPopup.java @@ -590,7 +590,12 @@ class CompletionProposalPopup implements IContentAssistListener { Listener listener= new Listener() { public void handleEvent(Event event) { - handleSetData(event); + try { + handleSetData(event); + } catch (RuntimeException e) { + unregister(); + throw e; + } } }; fProposalTable.addListener(SWT.SetData, listener);
(In reply to Terry Parker from comment #0) > As detailed in bug 418092, if a completion proposer throws a runtime > exception, the entire Eclipse UI can be held hostage. Yes, it destroys content assist for that editor. One has to close and reopen the editor. I agree we should fix this, but your proposed solution does not work for me. I think a better fix would put the exception into the label, given the UI wants the label on request. That way we prevent other proposals from not being shown and we also prevent holes in the list. Terry, would you be willing to provide a patch? If so, please also add your credentials to the copyright notice.
(In reply to Dani Megert from comment #1) > (In reply to Terry Parker from comment #0) > > As detailed in bug 418092, if a completion proposer throws a runtime > > exception, the entire Eclipse UI can be held hostage. > > Yes, it destroys content assist for that editor. One has to close and reopen > the editor. On gtk/Linux it is actually worse than that, the keyboard and mouse focus are consumed by an invisible window, so your only option is to kill Eclipse :-( > > I agree we should fix this, but your proposed solution does not work for me. > > I think a better fix would put the exception into the label, given the UI > wants the label on request. That way we prevent other proposals from not > being shown and we also prevent holes in the list. Agreed that your solution prevents holes in the list. I used a static message and error icon to populate the failed attempt to get the proposal text, and log the full error to the error log. > > Terry, would you be willing to provide a patch? If so, please also add your > credentials to the copyright notice. Done. https://git.eclipse.org/r/#/c/24239/
Ping. Any comments on the proposed patch?
Submitted with http://git.eclipse.org/c/platform/eclipse.platform.text.git/commit/?id=ea964c7ec2be671ce9035a5b2f05e368566a82fb I've add your credentials to the copyright notice of the properties file. I've also guarded the case when virtual table is not supported with http://git.eclipse.org/c/platform/eclipse.platform.text.git/commit/?id=ca57b300d79431c27b32ae2113919be546cc42d9
Thanks for the review, Dani!
