Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [cdt-dev] Timeouts for GDB/MI commands (Was: [DSF] Making Sequence more robust)

On Monday, October 24, 2011 17:56:03 Marc Khouzam wrote:
> > -----Original Message-----
> > From: cdt-dev-bounces@xxxxxxxxxxx
> > [mailto:cdt-dev-bounces@xxxxxxxxxxx] On Behalf Of Vladimir Prus
> > Sent: Saturday, October 22, 2011 5:55 AM
> > To: Pawel Piech; Mikhail Khodjaiants
> > Cc: cdt-dev@xxxxxxxxxxx
> > Subject: [cdt-dev] Timeouts for GDB/MI commands (Was: [DSF]
> > Making Sequence more robust)
> 
> Hi,
> 
> I think it would be great to add some timer infrastructure
> for commands sent to GDB.  Thanks for pushing this forward.
> Please see below for some comments.
> 
> > On Thursday, October 20, 2011 21:40:17 Pawel Piech wrote:
> > > > 2. While a general repository for outstanding RM's might
> > 
> > be a good idea,
> > 
> > > > what would you say if we start small, and implement the
> > 
> > solution that
> > 
> > > > would remember outstanding RequestMonitor instances for
> > 
> > some limited
> > 
> > > > cases (for example, 'target *-remote' commands in
> > 
> > DSF-GDB, and then
> > 
> > > > cancel those RequestMonitors after a timeout? This is somewhat
> > > > orthogonal from the initial goal of catching lost RMs, but seems
> > > > sufficiently related and immediately useful.
> > > 
> > > This is something that could be done at the service level,
> > 
> > where these
> > 
> > > commands are issued.  If we expect that the back-end may
> > 
> > never complete
> > 
> > > some commands then in those cases a timeout guard should even be
> > > required.  For example, we queue commands with the command
> > 
> > control, but
> > 
> > > we count on the process monitor to notify us if the back
> > 
> > end process
> > 
> > > dies.  At that point we complete any outstanding command in
> > 
> > queue with
> > 
> > > an error.  So our process monitor is the guard.
> > 
> > We discussed this approach internally (turned out to be
> > faster), and Mikhail has proposed approach like this:
> > 
> > 1. Add new interface:
> >     interface IMIControlTimeoutPolicy {
> >     
> >         void commandSend(CommandHandle);
> >         void commandDone(CommandHandle);
> >     
> >     }
> 
> ICommandListener has commandSent() and commandDone().
> How about using 'sendCommand' and 'processCommandReply' to avoid
> confusion?

Well, 'sendCommand' sounds like a methods that sends a command,
as opposed to a method that is notified that a command is sent.
Maybe, createTimeoutPolicy should just return ICommandListener?

> 
> > 2. Add new method AbstractMIControl.createTimeoutPolicy
> > (returning null by default).
> > Adjust the code so that when this method returns non-null,
> > the commandSend and commandDone
> > methods of the result are called by TxThread/RxThread as appropriate.
> 
> Sure, that mostly makes that code easy to override.
> The interesting part will be the code that handles the timers.
> 
> > 3. Make GDBControl.createTimeoutPolicy return a policy that
> > sets a timer whenever a command
> > is sent, and if the command is not done within a timeout,
> > just calls GDBControl.shutdown.
> 
> I wonder if such a solution is sufficient?
> Do we want to recover for a missing reply, or should it never
> happen and we can indeed just abort?
> 
> A more flexible solution, which may be overkill as it may never
> be triggered would be to allow for different behaviors
> for different MI commands and different timeouts as well
> (I think CDI has that already).
> 
> How about adding some methods to MICommand?  Something like:
>   getTimeout() // Returns timeout value for this MICommand
>   timeout(RequestMonitor)    // Called if the timer expires before
>                              // a reply has been received.  This method
>                              // will decide what to do with the RM
> 
> We'd put in default implementation in MICommand, but it would be possible
> to override them in individual commands, such as 'target *-remote'.
> 
> I'm imagining that the MICommand.timeout(RM) will deal with the RM
> so that the service that sent the command can decided what to do
> in that case.
> 
> But again, this may be overkill if we don't expect this to happen
> often or at all.
> 
> Are you trying to address a specific scenario or a general safety
> mechanism?

We care both for initial connection timeout (-target-select) and a case
where target goes away in the middle of debug session (wedged board).
We actually discussed individual handling for commands on internal IRC,
and Mikhail suggested this is not a good idea, since it was implemented
this way in CDI and just caused multiple error messages in case target
is gone. Obviously, trying to implement error recovery for all commands
is a major undertaking in all cases.

So, I would suggest not to complicate interface until we sure per-command
timeout policy is both useful and implementable.

What do you think?

-- 
Vladimir Prus
CodeSourcery / Mentor Graphics
+7 (812) 677-68-40


Back to the top