Community
Participate
Working Groups
We have multiple debug launch delegates and each one has a set of tabs. Some tabs are shared while others are not. When the user creates a new launch configuration, if there is no preferred launch delegate selected in the global preference, all the tabs of all the launch delegates will be created. In one of those tabs (CDI's CMainTab), we force a default launch delegate, which will then hide all the other tabs. I believe we still have a problem because all tabs have still had their setDefaults() method call and have potentially modified the configuration. An example scenario will illustrate this best. For simplicity, let's just assume we have a CDI delegate and a DSF delegate. When we create a new Debug config, setDefaults() of the Debugger tab for CDI will be called, and setDefaults() the Debugger tab for DSF will be called. Both these methods set attributes in the configuration. The problem happens when both methods set the _same_ attribute to different values. For example, if the DSF tab sets the ATTR_DEBUGGER_ID to a value valid only for DSF, it is possible that CDI will use that value and give an error. I have not confirmed this directly, but I came to this theory based on the following error reported by bug 312817: "An internal error occurred during: "Launching Hello_GDB.exe" org.eclipse.cdt.dsf.gdb.launching.GDBDebugger cannot be cast to org.eclipse.cdt.debug.core.ICDebugger. Stack trace: java.lang.ClassCastException: org.eclipse.cdt.dsf.gdb.launching.GDBDebugger cannot be cast to org.eclipse.cdt.debug.core.ICDebugger at org.eclipse.cdt.debug.internal.core.DebugConfiguration.createDebugger(DebugConfiguration.java:61) at org.eclipse.cdt.launch.internal.LocalCDILaunchDelegate.createCDISession(LocalCDILaunchDelegate.java:466) at org.eclipse.cdt.launch.internal.LocalCDILaunchDelegate.launchLocalDebugSession(LocalCDILaunchDelegate.java:145) at org.eclipse.cdt.launch.internal.LocalCDILaunchDelegate.launchDebugger(LocalCDILaunchDelegate.java:112) at org.eclipse.cdt.launch.internal.LocalCDILaunchDelegate.launch(LocalCDILaunchDelegate.java:72) at org.eclipse.debug.internal.core.LaunchConfiguration.launch(LaunchConfiguration.java:853) at org.eclipse.debug.internal.core.LaunchConfiguration.launch(LaunchConfiguration.java:702) at org.eclipse.debug.internal.ui.DebugUIPlugin.buildAndLaunch(DebugUIPlugin.java:923) at org.eclipse.debug.internal.ui.DebugUIPlugin$8.run(DebugUIPlugin.java:1126) at org.eclipse.core.internal.jobs.Worker.run(Worker.java:54) In this exception, a CDI launch delegate is trying to use a DSF debugger type, which could indicate that the ATTR_DEBUGGER_ID configuration attribute was set by the wrong Debugger tab. So, the real problem reported by this bug is that we currently have multiple launch delegates and their tabs using the same configuration attributes in a blind way. I think the simplest solution would be to never share launch attributes, but to instead have unique ones for each delegate. This may not be ideal because the launch configuration will be polluted (although safely) with unused attributes of the unused delegates. Another solution is to somehow make sure that the config attributes are set properly by the preferred launch delegate.
Created attachment 168744 [details] Proposed fix I found another solution. Currently, the default launch delegates are set for an individual launch configuration. This means that every time we create a launch config, all tabs get created, the CDI CMainTab sets the preferred launch delegate, and only relevant tabs are shown to the user. So, every time we create a launch config, we have the race condition of all tabs being created and modifying the launch config in their own way. What I suggest doing instead is to set the default launch delegate for the launch configuration _type_. This basically sets the global preference of preferred launch delegate. Once that preference is set, only the right tabs will be open, and we no longer have the race condition. Also, by setting the default on the launch configuration _type, we only need to do it once, instead of every time a launch config is created (assuming the user never set her global preference manually). Furthermore, since we don't need to wait for a launch config to be created to set our defaults, we can set the preferred launch delegates on the launch config type very early, so that everything is ready for when the user creates a launch configuration. I was thinking of setting those defaults at the start of one of the CDT plugins. I have tried setting the default in org.eclipse.cdt.launch.internal.ui.LaunchUIPlugin#start() but the plugin was sometimes only started when a launch config was being created, which was too late. I have found that things seem to work well when setting the launch defaults in org.eclipse.cdt.debug.core.CDebugCorePlugin#start(). Finally, I didn't see any UI way for the user to disable the global preference for the preferred launch delegates, so I think that once we have set that preference, we will be safe. This simple patch sets the preferred launch delegate for each of CDT's three launch configuration types for Debug mode and for our single Run mode launch configuration type. Although setting the default for Run mode was not necessary (we only have one delegate), I feel it is safer, in case another delegate is added.
Pawel, this is a change in the original fix you had done. Can you confirm it is a valid solution?
A side-effect I just noticed of this solution is that if the user changes its global preference for the preferred launch delegate, all his existing launch configurations will change (unless he had forced them to use a specific launch delegate). At first I thought that was bad, but thinking about it more, I find that it is probably a good thing: it allows the user to very easily switch between any of our debuggers. And in the case the user wants to use a different debugger for a single session, they can still do it the same way as before, by overriding the preferred delegate, just for that launch config.
Created attachment 168769 [details] More complete proposed fix I had forgotten to clean up CApplicationLaunchShortcut.java.
Created attachment 169142 [details] Committed fix This is the patch I committed to HEAD. It is the same as the last patch but simply moved the new method setDefaultLaunchDelegates() at the bottom of CDebugCorePlugin.
Fixed
(In reply to comment #2) > Pawel, this is a change in the original fix you had done. Can you confirm it > is a valid solution? Yes, I think it makes perfect sense. This is what I wanted to originally do in bug 262826, but somehow I didn't think of this simple workaround for the convoluted implementation in platform.
(In reply to comment #7) > Yes, I think it makes perfect sense. This is what I wanted to originally do in > bug 262826, but somehow I didn't think of this simple workaround for the > convoluted implementation in platform. It took me four different bugs and for different solution to come up with this one :-) Hopefully that will be the one.
*** cdt cvs genie on behalf of mkhouzam *** Bug 312997: Set the default launch delegate using the launch configuration type instead of individual launch configurations. This avoids having the tabs for all launch delegates be created every time, and causing race conditions. [*] CApplicationLaunchShortcut.java 1.7 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.debug.ui/src/org/eclipse/cdt/debug/internal/ui/launch/CApplicationLaunchShortcut.java?root=Tools_Project&r1=1.6&r2=1.7 [*] CDebugCorePlugin.java 1.38 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.debug.core/src/org/eclipse/cdt/debug/core/CDebugCorePlugin.java?root=Tools_Project&r1=1.37&r2=1.38 [*] CMainTab.java 1.85 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.launch/src/org/eclipse/cdt/launch/ui/CMainTab.java?root=Tools_Project&r1=1.84&r2=1.85