| Summary: | NPE in handlers | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | z_Archived | Reporter: | Nicolas Bros <nicolas.bros> | ||||||
| Component: | EMF-Facet | Assignee: | Nicolas Bros <nicolas.bros> | ||||||
| Status: | CLOSED FIXED | QA Contact: | |||||||
| Severity: | normal | ||||||||
| Priority: | P4 | CC: | Ed.Merks, ed, fabien.giquel, gdupe, modisco.web-inbox | ||||||
| Version: | unspecified | Flags: | nicolas.bros:
indigo+
Ed.Merks: pmc_approved+ fabien.giquel: review+ gdupe: review+ |
||||||
| Target Milestone: | --- | ||||||||
| Hardware: | PC | ||||||||
| OS: | Mac OS X - Carbon (unsup.) | ||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
|
Description
Nicolas Bros
Me too, and I didn't know that I had EMF Facet installed; maybe Papyrus does it. Anyway NPE still present in RC2. Shouldn't this be "major" and could shutdown activity be bypassed? Created attachment 196881 [details] Patch for Bug 340886 RemoveLine Handler Hi, Indeed, those NPE should have been taken care of. If I add a breakpoint on NPE and close my Eclipse, I only detect one handler with this problem. Here is a patch for org.eclipse.emf.facet.widgets.nattable.internal.handlers.RemoveLineHandler.java to catch this NPE and return false in isEnabled() method (a) I, Nicolas Guyomar, wrote 100% of the code I've provided. (b) I have the right to contribute the code to Eclipse. (c) I contribute the content under the EPL. (d) This contribution contains no Cryptography features. Regards, Nicolas Guyomar Catching NPEs from known causes is rather inefficient. The problem is that getActivePage() returns null, so why not test the null return, rather than inflict an unnecessary exception dispatch. Created attachment 196883 [details] Patch Bug 340886 NPE in handlers Hi Edward, You are absolutely right, I only wanted to minimize code modification for RC3. Here is a patch to remove unnecessary try/catch blocks from our handlers, and replace them with null test. CopyHandler.java CreateNewElementHandler.java DeleteHandler.java ExportCommandHandler.java RemoveLineHandler.java (a) I, Nicolas Guyomar, wrote 100% of the code I've provided. (b) I have the right to contribute the code to Eclipse. (c) I contribute the content under the EPL. (d) This contribution contains no Cryptography features. Regards, Nicolas Guyomar Hello Ed (Merks), I don’t know if this bug is major. No messages are sent to the log and no error pops up. The only way to check this bug is to add a breakpoint on java.lang.NullPointerException. I’ve checked the patch: it looks good and it does not break the unit tests. Is this bug major? Must we commit the patch before RC3? Regards, Gregoire (In reply to comment #5) > Is this bug major? No. I was forgetting that users don't see it. > Must we commit the patch before RC3? An easy fix, much better at RC3 than RC4. It's a pain for developers. I like to watch all NPE and CCE since they're the most insidious, and with spurious NPEs coming from handlers many interactive debug activities are hindered. If the risk of causing problems is low, then it seems good to fix it. Fabien, Please, can you check this patch ? Regards, Grégoire Comment on attachment 196883 [details] Patch Bug 340886 NPE in handlers This is a contribution from an employee of Mia-Software, targeting a future Indigo release. The company has signed a Member Committer Agreement. The contribution does not need a CQ (see bug 322327). Committed in revision 706. This bug can be closed. There are still NPEs in the RemoveLineHandler, that are invisible in Eclipse 3.x, but cause issues in 4.x. See Bug 354967 comment 7: https://bugs.eclipse.org/bugs/show_bug.cgi?id=354967#c7 I also get a NPE here: Thread [main] (Suspended (exception NullPointerException)) CreateNewElementHandler.isEnabled() line: 52 HandlerProxy.isEnabled() line: 320 Command.isEnabled() line: 833 Command.setHandler(IHandler) line: 965 HandlerAuthority.updateCommand(String, IHandlerActivation) line: 459 HandlerAuthority.processChangedCommands() line: 638 HandlerAuthority.access$1(HandlerAuthority) line: 610 HandlerAuthority$1.propertyChange(PropertyChangeEvent) line: 175 EvaluationAuthority$1.run() line: 252 SafeRunner.run(ISafeRunnable) line: 42 EvaluationAuthority.fireServiceChange(String, Object, Object) line: 246 EvaluationAuthority.endSourceChange(String[]) line: 197 EvaluationAuthority.sourceChanged(String[]) line: 135 EvaluationAuthority(ExpressionAuthority).sourceChanged(int, String[]) line: 311 EvaluationAuthority(ExpressionAuthority).sourceChanged(int, Map) line: 290 WorkbenchSourceProvider(AbstractSourceProvider).fireSourceChanged(int, Map) line: 99 WorkbenchSourceProvider.checkActivePart(boolean) line: 401 WorkbenchSourceProvider.checkActivePart() line: 300 WorkbenchSourceProvider.handleCheck(Shell) line: 286 WorkbenchSourceProvider.checkOtherSources(Shell) line: 855 WorkbenchSourceProvider$6.handleEvent(Event) line: 839 EventTable.sendEvent(Event) line: 84 Display.filterEvent(Event) line: 1262 Shell(Widget).sendEvent(Event) line: 1052 Shell(Widget).sendEvent(int, Event, boolean) line: 1077 Shell(Widget).sendEvent(int) line: 1058 Shell(Decorations).WM_ACTIVATE(long, long) line: 1647 Shell.WM_ACTIVATE(long, long) line: 2137 Shell(Control).windowProc(long, int, long, long) line: 4525 Shell(Canvas).windowProc(long, int, long, long) line: 341 Shell(Decorations).windowProc(long, int, long, long) line: 1610 Shell.windowProc(long, int, long, long) line: 2061 Display.windowProc(long, long, long, long) line: 4972 OS.DestroyWindow(long) line: not available [native method] Shell(Control).destroyWidget() line: 780 Shell.destroyWidget() line: 698 Shell(Widget).release(boolean) line: 818 Shell(Widget).dispose() line: 446 Shell(Decorations).dispose() line: 447 Shell.dispose() line: 715 SaveablesList$3(Window).close() line: 335 SaveablesList$3(Dialog).close() line: 979 SaveablesList$3(MessageDialog).buttonPressed(int) line: 207 Dialog$2.widgetSelected(SelectionEvent) line: 624 TypedListener.handleEvent(Event) line: 240 EventTable.sendEvent(Event) line: 84 Button(Widget).sendEvent(Event) line: 1053 Display.runDeferredEvents() line: 4165 Display.readAndDispatch() line: 3754 SaveablesList$3(Window).runEventLoop(Shell) line: 825 SaveablesList$3(Window).open() line: 801 SaveablesList$3(MessageDialog).open() line: 334 SaveablesList.promptForSaving(List, IShellProvider, IRunnableContext, boolean, boolean) line: 529 SaveablesList.promptForSavingIfNecessary(IShellProvider, IWorkbenchWindow, Set, Map, boolean) line: 433 SaveablesList.preCloseParts(List, boolean, IShellProvider, IWorkbenchWindow) line: 388 SaveablesList.preCloseParts(List, boolean, IWorkbenchWindow) line: 347 WorkbenchPage.closeEditors(IEditorReference[], boolean) line: 1426 WorkbenchPage.closeEditor(IEditorReference, boolean) line: 1514 EditorPane.doHide() line: 61 EditorStack(PartStack).close(IPresentablePart) line: 537 EditorStack.close(IPresentablePart[]) line: 206 PartStack$1.close(IPresentablePart[]) line: 120 TabbedStackPresentation$1.handleEvent(TabFolderEvent) line: 83 DefaultTabFolder(AbstractTabFolder).fireEvent(TabFolderEvent) line: 269 DefaultTabFolder(AbstractTabFolder).fireEvent(int, AbstractTabItem) line: 278 DefaultTabFolder.access$1(DefaultTabFolder, int, AbstractTabItem) line: 1 DefaultTabFolder$1.closeButtonPressed(CTabItem) line: 71 PaneFolder.notifyCloseListeners(CTabItem) line: 631 PaneFolder$3.close(CTabFolderEvent) line: 206 CTabFolder.onMouse(Event) line: 1598 CTabFolder$1.handleEvent(Event) line: 261 EventTable.sendEvent(Event) line: 84 CTabFolder(Widget).sendEvent(Event) line: 1053 Display.runDeferredEvents() line: 4165 Display.readAndDispatch() line: 3754 Workbench.runEventLoop(Window$IExceptionHandler, Display) line: 2696 Workbench.runUI() line: 2660 Workbench.access$4(Workbench) line: 2494 Workbench$7.run() line: 674 Realm.runWithDefault(Realm, Runnable) line: 332 Workbench.createAndRunWorkbench(Display, WorkbenchAdvisor) line: 667 PlatformUI.createAndRunWorkbench(Display, WorkbenchAdvisor) line: 149 IDEApplication.start(IApplicationContext) line: 123 EclipseAppHandle.run(Object) line: 196 EclipseAppLauncher.runApplication(Object) line: 110 EclipseAppLauncher.start(Object) line: 79 EclipseStarter.run(Object) line: 344 EclipseStarter.run(String[], Runnable) line: 179 NativeMethodAccessorImpl.invoke0(Method, Object, Object[]) line: not available [native method] NativeMethodAccessorImpl.invoke(Object, Object[]) line: 39 DelegatingMethodAccessorImpl.invoke(Object, Object[]) line: 25 Method.invoke(Object, Object...) line: 597 Main.invokeFramework(String[], URL[]) line: 622 Main.basicRun(String[]) line: 577 Main.run(String[]) line: 1410 Main.main(String[]) line: 1386 This bug has to be closed for the IP log. Sibnce this causes users considerable irritation, couldn't you fix it before closing it?? It seems very wrong to close an open bug. (In reply to comment #15) > Sibnce this causes users considerable irritation, couldn't you fix it before > closing it?? It seems very wrong to close an open bug. I hadn't any other choices: it is a constraint of the IP log generator, but I've open a new set of bugs : cf. https://bugs.eclipse.org/bugs/buglist.cgi?query_format=advanced;list_id=1737552;short_desc=NullPointerException;bug_status=UNCONFIRMED;bug_status=NEW;bug_status=ASSIGNED;bug_status=REOPENED;short_desc_type=allwordssubstr;product=EMFT.facet |