Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 345749 - [restart] MIInferior process gets terminated right away on a restart
Summary: [restart] MIInferior process gets terminated right away on a restart
Status: RESOLVED FIXED
Alias: None
Product: CDT
Classification: Tools
Component: cdt-debug-dsf-gdb (show other bugs)
Version: 8.0   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: 8.0   Edit
Assignee: Marc Khouzam CLA
QA Contact: Marc Khouzam CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-13 11:54 EDT by Marc Khouzam CLA
Modified: 2011-05-14 23:23 EDT (History)
2 users (show)

See Also:
marc.khouzam: review? (pawel.1.piech)

Attachments
Fix (11.64 KB, patch)
2011-05-14 22:33 EDT, Marc Khouzam CLA 		marc.khouzam: iplog-
Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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?