Community
Participate
Working Groups
Currently, DSF tracks the single inferior process. From what I understand, there are three reasons for that: 1- input/output console for the inferior 2- display the inferior in the debug view and ability to terminate it 3- tracking of the state of the inferior With multi-process debugging, there will be multiple inferior processes. Furthermore, the inferior processes will not be known in advance and will possibly change during the debug session (as the user attaches and detaches from inferiors.) Also, from what I can see, the first enhancements to MI will not allow to deal with different inferiors separately. For instance, killing a single inferior will probably not be supported; or redirecting the output of a single inferior will not be possible; instead, all inferiors will be affected at once. DSF/MI needs to slightly adapt for such things.
Created attachment 105060 [details] Adding a process to the launch from within the Executor (In reply to comment #0) > the inferior processes will not be known in advance and will > possibly change during the debug session (as the user attaches and detaches > from inferiors.) To allow to add the inferior process when the 'attach' command is given, there will be a need to call DebugPlugin.newprocess from within the Executor. Currently, this causes a deadlock since that call makes other calls to the Executor. This patch adds a check to see if we are already in the Executor. The change is simple, but I would appreciate a quick review to make sure I understood the Executor thing properly. Thanks
Created attachment 105116 [details] Second part of patch The GDBCLIProcess also needs to be prepared to be called from the Executor. I had to change a Callable to a Query to use the ImmediateExecutor. I believe the end result is the same.
If GDB does not support multiple inferior processes, maybe there's no need to rush to add this support in the client? As far as the patches, could you explain why these methods need to be called in the executor thread? In general I've been trying to avoid the if (in executor thread) kind of logic, because as you saw it's very easy to get into a deadlock situation.
(In reply to comment #3) > If GDB does not support multiple inferior processes, maybe there's no need to > rush to add this support in the client? GDB is actually working on this now. In fact, today, they posted patches to have multiple inferiors :-) However, I think we should discuss the inferior handling to see how exactly we want to deal with it for GDB/DSF. > As far as the patches, could you explain why these methods need to be called in > the executor thread? In general I've been trying to avoid the if (in executor > thread) kind of logic, because as you saw it's very easy to get into a deadlock > situation. It's been a while :-) and I'm not entirely sure these patches are necessary. Let's ignore them until we can come up with a way to proceed about the inferior. I have put this bug on the back burner because, as you pointed out, it is not a rush. I will eventually get back to it, and try to write some kind of proposal or at least more intelligent questions :-)
Sounds good. I really hope that GDB will support multi-process debugging in linux by their next release... otherwise it may be hard to demo all this work you've been putting in.
(In reply to comment #5) > Sounds good. I really hope that GDB will support multi-process debugging in > linux by their next release... otherwise it may be hard to demo all this work > you've been putting in. I'm hoping that too :-) But today, this post http://sourceware.org/ml/gdb-patches/2008-09/msg00257.html gave me more hope by stating: "We also have been working on multi-process + non-stop support for linux gdbserver, which builds on top of this support. That will come in later." As long as 'later' is not too late :-)
I've been breaking my head trying to figure out how to deal with the inferior launch process in two situations: 1- multiple-processes 2- attaching when the process name is not known at launch time The obvious solution was to remove the inferior launch process in those cases, but I thought that maybe I should look for a better solution. Then, for fun, I tried a CDI Attach session and get this: there is no inferior added to the launch!!! So since multi-process is currently only available for attach sessions, I suggest we do the same in DSF-GDB that: for an attach session, we simply don't add an inferior to the launch. To address our current features that are enabled by having an inferior see below: (In reply to comment #0) > Currently, DSF-GDB tracks the single inferior process. From what I > understand, there are three reasons for that: > 1- input/output console for the inferior > 2- display the inferior in the debug view and ability to terminate it > 3- tracking of the state of the inferior When we do an attach launch: 1- the input/output of the inferior is not in the DSF-GDB console anyway, since we didn't actually start the process. So, this is not a problem. 2- displaying the inferior in the debug view is probably not a good idea in the case of multi-process since there could be many inferiors and it just makes the debug view more crowded. Also, we already don't support the killing of an inferior during an attach session (as can be seen in GDBInferiorProcess.destroy().) So this is also not a problem 3- tracking the state of the inferior is used internally and not visible to the user. I can just work around that. Also not a problem. So, I'm gonna work on a patch to remove the inferior for attach launches.
Created attachment 127686 [details] Quick fix for attach sessions This change is extremely simple. It simply does not add the inferior process to the launch in the case of an attach session. This basically solves the problem if showing an inferior when we don't know its name, or when there are many inferiors. committed. However, I don't like the fact that for multiple processes, we still create this arbitrary MIInferior process. I could not clean that up easily though because GDB does not yet have a final solution for multi-process for Linux. Therefore, I'm leaving this bug open to cleanup the use of MIInferior when GDB has its final solution.
All right, back to this bug for multi-process. We discussed how to handle multiple inferiors at the multicore debug conference call of February 22, 2011, and here is an excerpt of the minutes (http://wiki.eclipse.org/CDT/MultiCoreDebugWorkingGroup/calls/minutes) about this topic: 1- "We don't necessarily need to show the inferior process in the Debug View. Both EDC and TI have removed that entry." The only value I see about showing the inferior in the debug view is that it allow the user to terminate it. However, for single process debugging, it has the same effect as terminating the entire session, so it does not add something useful. For multi-process, we may want to allow to terminate a single process. We can surely achieve this by handling the terminate action properly based on which process (container) is selected in the debug view. Therefore, we still don't need to show the inferior. Finally, for attach sessions we already don't show the inferior. For Post-mortem sessions, we don't show the inferior, but that was kind of a bug. I'm not sure yet if we want to show it in that case. 2- "Another option is to create a group of inferiors in the DV if there is more than one inferior, instead of not showing the inferiors" We won't go for this approach right away, but it could be a backup plan if we don't like the fell of not showing the inferiors. 3- "We need the inferior process for the IO console and to monitor the life of the process" To easily get the console for the IO, we need an inferior process for each debugged process of a multi-process session, and that inferior must be added to the launch (that does not mean it needs to be shown in the Debug view). If stop debugging one of those processes (detaching from it), we should probably remove it from the launch. If the process terminates, the session will be kept active if there are other processes, but we can probably keep the process as part of the launch, since it will be marked as terminated. 4- "If we don't show the inferior, we can still automatically select the proper console when selecting a process (container) instead of when selecting the inferior." I will prototype a solution that does not show the inferior in the debug view and see how that feels.
Created attachment 189712 [details] Don't show the inferior This preliminary patch simply does not show the inferior in the Debug view. The solution does not handle multiple inferiors yet, which is what I'm going to work on now.
I've opened a separate bug (Bug 338136) to deal with hiding the inferior in the debug view. I wanted to keep that effort and the one about dealing with multiple inferiors separate.
Created attachment 189844 [details] Preparing the output This patch is a first step towards supporting multiple inferiors. It moves 1- the setup of the PTY with GDB 2- the adding of the inferior to the launch, to when we startOrRestart the process instead. Currently both these operations were being done once when we launch. This change will allow to repeat the operations every time we start a new process in multi-process. The other advantage of this change is that it also works for a restart operation, which allowed me to cleanup the GdbRestartCommand class, which used to duplicate this code. API changes: GdbLaunch#addInferiorProcess() is removed. GDBControl.InferiorInputOutputInitStep is removed. GDBControl_7_0.InferiorInputOutputInitStep is removed.
(In reply to comment #12) > Created attachment 189844 [details] > Preparing the output Committed to HEAD.
*** cdt cvs genie on behalf of mkhouzam *** Bug 237308: Support for multiple inferior processes [*] SyncUtil.java 1.19 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/framework/SyncUtil.java?root=Tools_Project&r1=1.18&r2=1.19 [*] GdbRestartCommand.java 1.6 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/actions/GdbRestartCommand.java?root=Tools_Project&r1=1.5&r2=1.6 [*] GDBControl_7_0.java 1.26 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/command/GDBControl_7_0.java?root=Tools_Project&r1=1.25&r2=1.26 [*] GDBControl.java 1.22 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/command/GDBControl.java?root=Tools_Project&r1=1.21&r2=1.22 [*] GdbLaunchDelegate.java 1.21 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/launching/GdbLaunchDelegate.java?root=Tools_Project&r1=1.20&r2=1.21 [*] GdbLaunch.java 1.13 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/launching/GdbLaunch.java?root=Tools_Project&r1=1.12&r2=1.13 [*] StartOrRestartProcessSequence_7_0.java 1.4 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/StartOrRestartProcessSequence_7_0.java?root=Tools_Project&r1=1.3&r2=1.4 [*] GDBProcesses.java 1.14 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBProcesses.java?root=Tools_Project&r1=1.13&r2=1.14
Created attachment 189956 [details] Plugin to add an action to create a new process This is a test plugin to allow to create a new process for multi-process debugging. It adds a menu in the top menu.
Created attachment 190611 [details] Cleanup of MIInferior Here is a patch that cleans up MIInferiorProcess and makes is simply handle what it should, which is the methods inherited for java.lang.Process. I'll write up more details later on, but I wanted to store the patch. I still have to run more tests also.
Created attachment 190612 [details] Cleanup of MIInferior (try 2) (In reply to comment #16) > Created attachment 190611 [details] > Cleanup of MIInferior It's late, I attached the wrong patch. Here is the correct one.
Created attachment 190643 [details] Cleanup of MIInferior (try 3) I found a bug when doing an attach for GDB < 7.0. In that case, we were no longer sending the ContainerExitedDMEvent. Fixed with this patch.
Created attachment 190809 [details] Cleanup of MIInferior committed to HEAD I committed this patch to HEAD. It is the same as the previous one except I got rid of some state in the MIInferiorProcess that was no longer being used.
The patch I committed had two aspects to it. #1- Make MIInferiorProcess as 'dumb' as possible. Many of the things it used to do, don't belong there once we start talking about multi-process. I got rid of the inferior state of Running/Stopped and only deal with a boolean for terminated. The inferior running or not was meant to know if we could send commands to GDB. With non-stop or with multi-process, this did not make sense anymore, and I moved that logic to the IMIRunControl as part of bug 339047. The inferior no longer holds its pid. This was only useful to allow us an easy access to the pid. Instead, since the pid is actually part of the context hierarchy, I just made sure that whenever we used to request the pid from the inferior, we now have proper access from the context itself. The constructors of MIInferiorProcess now take an IContainerDMC as a parameter since there could be multiple inferiors. So now, MIInferiorProcess is used for two things: for the IO console, and to fetch the exitCode. See bug 338136 for limitations about the exitCode. Since MIInferiorProcess is only needed for IO, it is now only created for inferiors for which we have access to the IO; that is to say that we no longer create an MIInferiorProcess for processes we attach to, or for processes running remotely, or for post-mortem sessions. A side-effect of MIInferiorProcess being 'dumb' is that we never need to access it once it is running. So, there is no method to getInferiorProcess() anymore; instead, MIInferiorProcess is added to the launch and that is it. #2- The creation of the PTY, setting it in GDB and the creation of the MIInferiorProcess are no longer done by IGDBControl. These steps must be done for each process and therefore belong in the IProcesses services. As for API breakage: 1- IGDBControl, GDBControl and GDBControl_7_0, no longer have the three methods: initInferiorInputOutput(), createInferiorProcess() and getInferiorProcess() 2- MIInferiorProcess's constructors have changed, and many of its public methods are removed (getState(), getPid(), setPid(), etc) 3- GdbInferiorProcess no longer exists (this was done in another patch)
*** cdt cvs genie on behalf of mkhouzam *** Bug 237308: Support for multiple inferior processes [*] GDBControl_7_0.java 1.30 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/command/GDBControl_7_0.java?root=Tools_Project&r1=1.29&r2=1.30 [*] IGDBControl.java 1.10 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/command/IGDBControl.java?root=Tools_Project&r1=1.9&r2=1.10 [*] GDBControl.java 1.26 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/command/GDBControl.java?root=Tools_Project&r1=1.25&r2=1.26 [*] StartOrRestartProcessSequence_7_0.java 1.7 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/StartOrRestartProcessSequence_7_0.java?root=Tools_Project&r1=1.6&r2=1.7 [*] GDBProcesses.java 1.19 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBProcesses.java?root=Tools_Project&r1=1.18&r2=1.19 [*] GDBRunControl.java 1.17 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBRunControl.java?root=Tools_Project&r1=1.16&r2=1.17 [*] GDBProcesses_7_0.java 1.41 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBProcesses_7_0.java?root=Tools_Project&r1=1.40&r2=1.41 [*] MIInferiorTTYSet.java 1.3 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/commands/MIInferiorTTYSet.java?root=Tools_Project&r1=1.2&r2=1.3 [*] MIRunControlEventProcessor.java 1.14 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/MIRunControlEventProcessor.java?root=Tools_Project&r1=1.13&r2=1.14 [*] CommandFactory.java 1.24 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/CommandFactory.java?root=Tools_Project&r1=1.23&r2=1.24 [*] MIInferiorProcess.java 1.13 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/MIInferiorProcess.java?root=Tools_Project&r1=1.12&r2=1.13 [*] MIProcesses.java 1.11 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/MIProcesses.java?root=Tools_Project&r1=1.10&r2=1.11
Mikhail, can you review?
*** cdt cvs genie on behalf of mkhouzam *** Bug 237308: Support for multiple inferior processes. No need to have this method public. [*] GDBProcesses.java 1.20 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBProcesses.java?root=Tools_Project&r1=1.19&r2=1.20
