Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 345749

Summary: [restart] MIInferior process gets terminated right away on a restart
Product: [Tools] CDT Reporter: Marc Khouzam <marc.khouzam>
Component: cdt-debug-dsf-gdbAssignee: Marc Khouzam <marc.khouzam>
Status: RESOLVED FIXED QA Contact: Marc Khouzam <marc.khouzam>
Severity: normal    
Priority: P3 CC: cdtdoug, pawel.1.piech
Version: 8.0Flags: marc.khouzam: review? (pawel.1.piech)
Target Milestone: 8.0   
Hardware: PC   
OS: Linux   
Whiteboard:
Attachments:
Description Flags
Fix marc.khouzam: iplog-

Description Marc Khouzam CLA 2011-05-13 11:54:27 EDT 
The MIInferiorProcess class automatically calls terminate() and closes its streams when getting an IExitedDMEvent for a process.

On a restart, GDB creates the new process before indicating that the previous one was terminated.  So, the IExitedDMEvent is received by both the old MIInferiorProcess and the new MIInferiorProcess, and they both think they should call terminate() because they both have the same id.

The consequence is that after a restart, the streams of the inferior process are closed right away.
Comment 1 Marc Khouzam CLA 2011-05-13 14:05:15 EDT 
This problem happens for GDB >= 7.0 only right now.

It seems that for GDB < 7.0, we don't even receive the IExitedDMEvent which is problem in itself.  Once the event is received, the immediate terminate() might also affect those versions.
Comment 2 Marc Khouzam CLA 2011-05-13 14:16:38 EDT 
(In reply to comment #0)

> The consequence is that after a restart, the streams of the inferior process
> are closed right away.

Actually, the process can't even run very long after the restart and gets a SIGHUP because of this issue.
Comment 3 Marc Khouzam CLA 2011-05-14 22:33:28 EDT 
Created attachment 195655 [details]
Fix

This fix does two things:

when doing a restart, it replaces the containerDmc for the original process, with a new one that will represent the new process.  We do this because the context hierarchy contains the pid, and the pid will change once the process is restarted, so we should not re-use the same container.  The trick was to choose the right groupId based on the GDB version.

GDB <= 6.8, the groupId is the MIProcesses.UNIQUE_GROUP_ID, as it always is.  
GDB 7.0 and 7.1, the groupId will eventually be the new pid, but until the process is started, we have to fall back on MIProcesses.UNIQUE_GROUP_ID (this is another reason why we can't use the old container, which will have the groupId set to the pid of the original process)
GDB >= 7.2, the groupId remains the same even after the restart.

The second thing this patch does is to have MIInferiorProcess listen for IStartedEvent for the process.  This allows us to get the fully formed Container context with pid, once the process is started, but also, we mark MIInferiorProcess as 'started'.  Now that we know if an inferior has been started yet, we can ignore the IExitedDMEvent for the new inferior that was not started yet.

I admit that things are getting pretty complex.  The fundamental reason for that is that we support many different versions of GDB that have subtle differences.
Comment 4 Marc Khouzam CLA 2011-05-14 22:47:30 EDT 
Committed to HEAD.
Comment 5 Marc Khouzam CLA 2011-05-14 22:54:43 EDT 
Pawel, can you review?