Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 402677 - NPE when no tool selected with new ETFw w/ JAXB parsing
Summary: NPE when no tool selected with new ETFw w/ JAXB parsing
Status: CLOSED FIXED
Alias: None
Product: PTP
Classification: Tools
Component: ETFw (show other bugs)
Version: 7.0   Edit
Hardware: PC All
: P3 normal (vote)
Target Milestone: 7.0   Edit
Assignee: Chris Navarro CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-07 12:13 EST by Brian Watt CLA
Modified: 2013-06-11 13:53 EDT (History)
1 user (show)

See Also:


Attachments
patches etfw tool launch delegate (10.60 KB, patch)
2013-03-08 16:57 EST, Chris Navarro CLA
g.watson: iplog+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brian Watt CLA 2013-03-07 12:13:08 EST
I did a 'run' launch and pointed to a simple script that dumps out the env vars
that works fine and I see all the results, tthat is, I see a list of env vars. Then I immediately do a 'profile launch' and get a NPE, because no tool has been set on the perf analysis tab I think.

!ENTRY org.eclipse.core.jobs 4 2 2013-03-07 11:00:34.775
!MESSAGE An internal error occurred during: "Launching envit".
!STACK 0
java.lang.NullPointerException
       at java.util.TreeMap.getEntry(TreeMap.java:324)
       at java.util.TreeMap.get(TreeMap.java:255)
       at org.eclipse.ptp.etfw.jaxb.util.JAXBExtensionUtils.getTool(JAXBExtensionUtils.java:55)
       at org.eclipse.ptp.etfw.internal.ToolLaunchManager.launchJAXBTool(ToolLaunchManager.java:122)
       at org.eclipse.ptp.etfw.internal.ToolLaunchManager.launch(ToolLaunchManager.java:110)
       at org.eclipse.ptp.etfw.parallel.ParallelToolLaunchConfigurationDelegate.launch(ParallelToolLaunchConfigurationDelegate.java:94)
       at org.eclipse.debug.internal.core.LaunchConfiguration.launch(LaunchConfiguration.java:855)
       at org.eclipse.debug.internal.core.LaunchConfiguration.launch(LaunchConfiguration.java:704)
       at org.eclipse.debug.internal.ui.DebugUIPlugin.buildAndLaunch(DebugUIPlugin.java:1018)
       at org.eclipse.debug.internal.ui.DebugUIPlugin$8.run(DebugUIPlugin.java:1222)
       at org.eclipse.core.internal.jobs.Worker.run(Worker.java:53)

If I then go back into the Performance Analysis tab and set a tool the profile launch works OK, no NPE.
Comment 1 Brian Watt CLA 2013-03-07 12:25:32 EST
Test the following case: Assume that there are no extensions defined for the extension point "org.eclipse.ptp.etfw.jaxb.workflows" anywhere. In that case the same NPE problem occurs and there is no work-around, because there are no tool to select in "Select tool:" dtop-down list.
Comment 2 Chris Navarro CLA 2013-03-08 10:21:06 EST
I think I have a good idea on how to fix this. I'll add a patch shortly.
Comment 3 Chris Navarro CLA 2013-03-08 12:41:31 EST
Digging into this further, I'm not sure what the right way to handle this is. Once you create a run configuration and execute it, the profile menu is populated since it is simply building on the run configuration. At that point you can run "something". Now, the reason for the error in the JAXB was because the launch manager that executes the workflow steps made the assumption that if the sax parser was not selected in the UI, then the user must have selected jaxb. I've made this smarter to check specifically in the launch configuration to see if the jaxb parser or sax parser was selected, then try launch their respective workflow steps. However, this only eliminates the null pointer exception. If the user never brought up the profile configuration wizard to make selections, then the launch will attempt to run, see no parser is selected and then just stop. Since this is in a UI plugin, I could popup an error dialog to inform the user that no profiling tool selections were made. I need advice on whether this is an acceptable solution.

I have also made the PerformanceAnalysisTab smarter so that based on UI selections it will show error dialogs for both sax and jaxb parsers if the user hasn't selected enough in the UI to try and run the job (e.g. no tool selected).
Comment 4 Brian Watt CLA 2013-03-08 13:41:29 EST
> If the user never brought up the
> profile configuration wizard to make selections, then the launch will
> attempt to run, see no parser is selected and then just stop. Since this is
> in a UI plugin, I could popup an error dialog to inform the user that no
> profiling tool selections were made. I need advice on whether this is an
> acceptable solution.

I would assume that if the user never brought up the profile configuration wizard then the "profile" launch should not stop, but execute as if it were a "run" launch. So I would not expect a pop-up error dialog.
Comment 5 Chris Navarro CLA 2013-03-08 13:56:40 EST
I think this requires a discussion with at least Greg because this is outside of my code and into the original ETFw framework (ParallelToolLaunchConfigurationDelegate) which extends PTP's ParallelLaunchConfigurationDelegate and performs the "launch". When the ETFw launch delegate calls the ToolLaunchManager, it expects a workflow to be present (which there is none if you don't run the profile configuration wizard). I don't know enough about the launch process to subvert it if no parser is selected (presumably we'd do this inside the ETFw launch delegate). 

On a side note, I'm not sure I agree that running the executable is correct either because this makes it appear as if everything went OK when in fact something was missing (a profiling tool). A user might wonder where their profiling information is since something happened (it ran the executable). A more novice user might not realize that they needed to take some additional actions. At least the error dialog would prompt them in the right direction (open the run profile wizard, select a profile tool).
Comment 6 Chris Navarro CLA 2013-03-08 14:13:50 EST
Looking at the debugger, you can actually do the same thing as profiling (launch without running the debug wizard). However, the launch delegate checks for the debugger path and throws a CoreException if the path is not set. I think this might be the desired behavior for profiling as well. I can check in the launch delegate to see if a profiling tool is selected and if not, throw a core exception.

Thoughts?
Comment 7 Greg Watson CLA 2013-03-08 14:44:45 EST
First, I don't really like the selection of the ETFw version on the configuration page. To me this is an implementation issue and should be moved to a preference page.

If the user has not configured the profile launch, then it should not be launchable. i.e. should not appear in the launch menu. If this is not possible, then I think you will need to show an error dialog. I think that is preferable to running the application, since this would be unexpected behavior.
Comment 8 Chris Navarro CLA 2013-03-08 15:02:49 EST
Making this a preference would not be hard and if that's preferable then I'll do it (please let me know here or open a bug). I thought this would be simpler for users to switch between them if they are in the middle of converting workflows from sax to jaxb. Presumably there will only be 1 framework eventually which is why I didn't go with a preference. 

As for appearing in the launch menu, the debugger has the same problem. If you create a run configuration and run an application, then you can immediately run the debugger because it shows up in the run list for the debugger menu. I don't know of any way to prevent this except to re-work our launch delegates so run is completely separate from debug which is completely separate from profile. But that would make it painful to have to keep setting up new launch configurations for the same machine to do different tasks.

That being said, I have a patch which does exactly what the debugger launch delegate does, it checks to see if a profiling tool is selected (similar to debugger path) and throws a core exception with additional information (open profiling configuration and check performance analysis tab).  

(In reply to comment #7)
> First, I don't really like the selection of the ETFw version on the
> configuration page. To me this is an implementation issue and should be
> moved to a preference page.
> 
> If the user has not configured the profile launch, then it should not be
> launchable. i.e. should not appear in the launch menu. If this is not
> possible, then I think you will need to show an error dialog. I think that
> is preferable to running the application, since this would be unexpected
> behavior.
Comment 9 Greg Watson CLA 2013-03-08 16:45:23 EST
Please attach the patch. I'll open a bug for the menu issue.
Comment 10 Chris Navarro CLA 2013-03-08 16:57:21 EST
Created attachment 228161 [details]
patches etfw tool launch delegate

The ETFw tool launch delegate did not provide an implementation of verifyLaunchAttributes that would prevent it from launching without a performance tool being selected. This patch implements the method and checks to make sure a tool has been selected. If not, it throws a CoreException to with a message to open the profile configuration and check the performance analysis tab.
Comment 11 Greg Watson CLA 2013-03-12 12:14:05 EDT
Committed to master. Please verify that it works as expected.
Comment 12 Chris Navarro CLA 2013-03-12 14:14:06 EDT
This is working for me.
Comment 13 Brian Watt CLA 2013-03-12 14:50:42 EDT
Working for me too. Closing as resolved/fixed
Comment 14 Brian Watt CLA 2013-06-11 13:53:02 EDT
Closing.