Bug 78816 - terminate session when GDB is not connected to the target causes error
Summary: terminate session when GDB is not connected to the target causes error
Status: RESOLVED FIXED
Alias: None
Product: CDT
Classification: Tools
Component: cdt-debug (show other bugs)
Version: 2.1   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.0   Edit
Assignee: Alain Magloire CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
: 77914 (view as bug list)
Depends on:
Blocks:
 
Reported: 2004-11-17 03:07 EST by Oyvind Harboe CLA
Modified: 2008-06-18 19:06 EDT (History)
1 user (show)

See Also:


Attachments
Fixes the problem by ignoring errors in kill and proceeding with the quit (1.79 KB, patch)
2004-11-17 03:08 EST, Oyvind Harboe CLA
bjorn.freeman-benson: iplog+
Details | Diff
New attempt at correctly determining the state of the target (908 bytes, patch)
2004-11-18 03:00 EST, Oyvind Harboe CLA
bjorn.freeman-benson: iplog+
Details | Diff
Fixes terminate problems by ignoring state of debugger and always executing the "kill" command ignore result + "exit" (2.01 KB, patch)
2004-11-19 09:50 EST, Oyvind Harboe CLA
bjorn.freeman-benson: iplog+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Oyvind Harboe CLA 2004-11-17 03:07:23 EST
Attached patch fixes the problem on my rocket.

Øyvind
Comment 1 Oyvind Harboe CLA 2004-11-17 03:08:16 EST
Created attachment 15916 [details]
Fixes the problem by ignoring errors in kill and proceeding with the quit

Trace after applying patch.


[1,100,678,734,484] (gdb) 
[1,100,678,742,063] 22 kill
[1,100,678,742,063] &"kill\n"
[1,100,678,742,063] &"The program is not being run.\n"
[1,100,678,742,063] 22^error,msg="The program is not being run."
[1,100,678,742,063] (gdb) 
[1,100,678,744,422] 23-gdb-exit
[1,100,678,744,422] 23^exit
Comment 2 Alain Magloire CLA 2004-11-17 10:42:22 EST
> Fixes the problem by ignoring errors in kill and proceeding with the quit
> Trace after applying patch.

8-)
Well I do not like to ignore exceptions blindly.
The question we should ask is why are we trying to
kill a programming that was not even running.

I put a tentative fix in the head branch a few minutes ago
Give a try.  Basically we should only do this(call abort/kill)
when the program is suspended any other state will not work.

Please give it a try.
Comment 3 Alain Magloire CLA 2004-11-17 10:42:51 EST
in the head
Comment 4 Alain Magloire CLA 2004-11-17 10:43:24 EST
please verify when you have a chance.
Comment 5 Oyvind Harboe CLA 2004-11-17 10:57:10 EST
Still broken.

Your fix does not handle the "disconnected" state.

This can happen if I start a remote debug session against a target that exists,
but does not respond to the TCP/IP requests.

Øyvind
Comment 6 Oyvind Harboe CLA 2004-11-17 11:33:54 EST
There is something deeply troubling here.

Your code change is trying to anticipate the result of a GDB command based upon
what it believes the state of the debugger to be. 

Can that ever end well?

isSuspended() is not 100% relieable, ref. other open PR's.

Just thinking aloud here:

Should CDT try to track the state of CDT at all?

The alternative would be to simply pass along "step", "stop", "suspend", etc.
commands directly to GDB and let GDB decide what to do.

Øyvind




Øyvind
Comment 7 Alain Magloire CLA 2004-11-17 11:44:49 EST
> Still broken.

How ?

> Your fix does not handle the "disconnected" state.

Why is the logic wrong, see MIInferior.terminate() :

		if ((session.isAttachSession() && isConnected()) ||     
(session.isProgramSession() && !isTerminated())) {
			// Try to interrupt the inferior, first.
			if (isRunning()) {
				interrupt();
			}
			int token = 0;
			if (isSuspended()) {
				CommandFactory factory = session.getCommandFactory();
				MIExecAbort abort = factory.createMIExecAbort();
				session.postCommand0(abort, session.getCommandTimeout());
				abort.getMIInfo();
				token = abort.getToken();
			}
			setTerminated(token, true);
		} else if (session.isCoreSession() && !isTerminated()){
			setTerminated();
		}

> This can happen if I start a remote debug session against a target that exists,
> but does not respond to the TCP/IP requests.

If a target does not respond, why would you want to discard
the exception, it is valuable information.

> The alternative would be to simply pass along "step", "stop", "suspend", etc.
> commands directly to GDB and let GDB decide what to do.

We can, we partly have to track the state, since some commands
will not be accepted depending on the state.  For example
we can not issue "continue" before a "run", we can not
issue a "run" if the target was attached, wen can not issue
"kill/abort" if we are not suspended.

The situation is not as dramatic, we do a good job of keeping
track.  When things get hairy/difficult is when the user
send commands to gdb without CDT knowing about it, this is
one reason we do like the gdb console 8-).

But GDB/MI is trying to do a good job in notifyin the frontend
of change states.





Note moving this to 3.0, it is not a priority that should stall
CDT-2.1
Comment 8 Oyvind Harboe CLA 2004-11-17 11:59:16 EST
>> Still broken.
>
>How ?

The session isn't terminated. See bottom of this message.

>> Your fix does not handle the "disconnected" state.
>
>Why is the logic wrong, see MIInferior.terminate() :

I haven't investigated in detail, but I believe the "kill" command fails because
 GDB isn't connected to the target.

>We can, we partly have to track the state, since some commands
>will not be accepted depending on the state.  For example
>we can not issue "continue" before a "run", we can not
>issue a "run" if the target was attached, wen can not issue
>"kill/abort" if we are not suspended.

In this case, CDT would of course have to simply in forward the error messages
in some non-obnoxious way(i.e. no modal dialog boxes :-)

>The situation is not as dramatic, we do a good job of keeping
>track.  When things get hairy/difficult is when the user
>send commands to gdb without CDT knowing about it, this is
>one reason we do like the gdb console 8-).
>
>But GDB/MI is trying to do a good job in notifyin the frontend
>of change states.

I'm not suggesting it is a good idea to change CDT, or that I agree with my own
line of thoughts :-), but I thought it valuable to air some ideas.

>Note moving this to 3.0, it is not a priority that should stall
>CDT-2.1

How about always ignoring exceptions from "kill" command? 

That would be a good stop-gap, and possible even the right solution.

Øyvind




[1,100,710,712,828] 16 info remote-process
[1,100,710,712,844] &"info remote-process\n"
[1,100,710,712,844] &"Undefined info command: \"remote-process\".  Try \"help
info\".\n"
[1,100,710,712,844] 16^error,msg="Undefined info command: \"remote-process\". 
Try \"help info\"."
[1,100,710,712,844] (gdb) 
[1,100,710,712,844] 17 info program
[1,100,710,712,859] &"info program\n"
[1,100,710,712,859] ~"The program being debugged is not being run.\n"
[1,100,710,712,859] 17^done
[1,100,710,712,859] (gdb) 
[1,100,710,713,016] 18-environment-cd C:\eclipse\workspace\performance
[1,100,710,713,016] 18^done
[1,100,710,713,016] 19 info threads
[1,100,710,713,016] (gdb) 
[1,100,710,713,016] &"info threads\n"
[1,100,710,713,031] &"No registers.\n"
[1,100,710,713,031] 19^error,msg="No registers."
[1,100,710,713,031] (gdb) 
[1,100,710,714,203] 20 kill
[1,100,710,714,219] &"kill\n"
[1,100,710,714,219] &"The program is not being run.\n"
[1,100,710,714,219] 20^error,msg="The program is not being run."
[1,100,710,714,219] (gdb) 
[1,100,710,719,219] 21 kill
[1,100,710,719,219] &"kill\n"
[1,100,710,719,219] &"The program is not being run.\n"
[1,100,710,719,219] 21^error,msg="The program is not being run."
[1,100,710,719,219] (gdb) 
[1,100,710,719,281] 22 kill
[1,100,710,719,281] &"kill\n"
[1,100,710,719,281] &"The program is not being run.\n"
[1,100,710,719,281] 22^error,msg="The program is not being run."
[1,100,710,719,281] (gdb) 
[1,100,710,719,281] 23-gdb-exit
[1,100,710,719,297] 23^exit
Comment 9 Alain Magloire CLA 2004-11-17 18:58:04 EST
>> Still broken.
>
>How ?

> The session isn't terminated. See bottom of this message.

The case below should have worked.
The target was not connected, not attached, nor suspended, nor running.

So, the kill command should not have been issued.
We have code in MIInferior.terminate() especially for that.

If the kill was issued that mean CDT/MI new about the state
and decide that the inferior was loaded, that it should "kill" it.

Did you set the state explicitely somewhere to suspended ?

In brief they should be no "kill"(See the head branch)
since MIInferior.state == 0


Comment 10 Oyvind Harboe CLA 2004-11-18 03:00:19 EST
Created attachment 15958 [details]
New attempt at correctly determining the state of the target

As you pointed out, CDT needs to know the state of the target.

In my experience, it is a very serious matter that CDT gets this wrong, because
a lot of assumptions are built on top of this.

Embedded targets do not necessarily support "info remote-process".

This patch also fixes: 

https://bugs.eclipse.org/bugs/show_bug.cgi?id=77914

Øyvind
Comment 11 Alain Magloire CLA 2004-11-18 12:39:13 EST
> Embedded targets do not necessarily support "info remote-process".
> This patch also fixes: 
> https://bugs.eclipse.org/bugs/show_bug.cgi?id=77914

Very interesting how about instead
"info stack"

using "info registers" is to dangerous since it will probe
for __all__ register values, that can be a dangerous thing for some
boards

So I'll give "info stack" a try.  Does it work for you
if yes, we'll move this to head and branch.
Comment 12 Alain Magloire CLA 2004-11-18 13:01:38 EST
Use "info stack" instead,
one more try .. flip to fix 8-)
Comment 13 Oyvind Harboe CLA 2004-11-18 14:24:10 EST
Phew! :-)
Comment 14 Oyvind Harboe CLA 2004-11-18 14:25:16 EST
*** Bug 77914 has been marked as a duplicate of this bug. ***
Comment 15 Oyvind Harboe CLA 2004-11-19 09:47:13 EST
Arrgghhh....

I don't understand why yet, but GDB dropped out of the "suspended" ->
"disconnected", making it impossible to terminate the session.




Øyvind
Comment 16 Oyvind Harboe CLA 2004-11-19 09:50:02 EST
Created attachment 16015 [details]
Fixes terminate problems by ignoring state of debugger and always executing the "kill" command ignore result + "exit"
Comment 17 Oyvind Harboe CLA 2004-11-19 09:54:35 EST
To reproduce:

- while in a debug session touch one of the source files to cause an automatic
rebuild
- this will delete the executable "testgcc.exe"

This you can see this confuses CDT and there is a disconnected "stuck" CDT
session in the Debug view.

Hmmm.... the "kill" command is executed twice...


[1,100,875,820,578] (gdb) 
[1,100,875,833,812] 78 kill
[1,100,875,833,812] &"kill\n"
[1,100,875,833,812] &"/ecos-c/eclipse/workspace/testgcc/Debug/testgcc.exe: No
such file or directory\
.\n"
[1,100,875,833,812]
78^error,msg="/ecos-c/eclipse/workspace/testgcc/Debug/testgcc.exe: No such file \
or directory."
[1,100,875,833,828] (gdb) 
[1,100,875,847,046] 79 kill
[1,100,875,847,062] &"kill\n"
[1,100,875,847,062] &"The program is not being run.\n"
[1,100,875,847,062] 79^error,msg="The program is not being run."
[1,100,875,847,062] (gdb) 
[1,100,875,853,687] 80-gdb-exit
[1,100,875,853,687] 80^exit
Comment 18 Alain Magloire CLA 2004-11-23 10:51:21 EST
> - while in a debug session touch one of the source files to cause an 
automatic
rebuild
> - this will delete the executable "testgcc.exe"

First, I'm not sure how you manage to get this on Windows XP
since if the debugger opens the file, it is not possible to
erase/delete/overwrite the executable while the debugger has the lock.
Actually the build should have failed.

Second I'm not convice that blindly ignoring error notifications by
the back-end debugger is a good thing.  What I expect is the following
workflow:

- try to kill program
- Error message(popup) things did not work out
- User looks at error.
- The user selects the __ debug session__ and  hits terminate
  the session __will__ go away.

> This you can see this confuses CDT and there is a disconnected "stuck" CDT
> session in the Debug view.

Try to select the "session" instead of the target, and the session should
terminate.  If you can not do this .. the yes we have a bug
Cc:ing mikhailk, perhaps he knows about the state problems

Comment 19 Oyvind Harboe CLA 2004-11-23 11:01:41 EST
>> - while in a debug session touch one of the source files to cause an 
>>automatic
>>rebuild
>> - this will delete the executable "testgcc.exe"
>
>First, I'm not sure how you manage to get this on Windows XP
>since if the debugger opens the file, it is not possible to
>erase/delete/overwrite the executable while the debugger has the lock.
>Actually the build should have failed.

The build did fail, and the testgcc.exe file was deleted. I'm using CygWin.
CygWin does not open files in the same fashion as your average Windows program.

>Second I'm not convice that blindly ignoring error notifications by
>the back-end debugger is a good thing.  

Sometimes it is a good idea. CDT does do it in other places.

Will test some more.

Øyvind
Comment 20 Oyvind Harboe CLA 2004-11-23 11:07:52 EST
>Second I'm not convice that blindly ignoring error notifications by
>the back-end debugger is a good thing. 

What makes this  a bit icky, is that CDT is trying to guess the state of the
debugger.

If CDT gets it wrong, a debug session is "stuck".

"Blindly ignoring the error" and always executing the "exit" command afterwards
has the following advantages:

- I can't think of  a scenario where the user cares why the "kill" request
failed. There exists logging capabilities to track such obscure problems.
- CDT does not have to guess the state of GDB correctly


I can't think of any problems worthwhile of mentioning w.r.t. "blindly ignoring"
a kill command. Except that it might be symptomatic of a problem in CDT, which
the user wouldn't care about, unless this internal state handling problem
manifests itself in some obseravable manner.

I'll do some more testing now.

Øyvind
Comment 21 Oyvind Harboe CLA 2004-11-23 11:24:02 EST
>Try to select the "session" instead of the target, and the session should
>terminate.  If you can not do this .. the yes we have a bug
>Cc:ing mikhailk, perhaps he knows about the state problems

If I terminate on the top level of the Debug session tree, it works.

If I terminate further down the debug tree, I get an error message + terminate
does not work.

I didn't realize that there exists two different types of Terminate. 

AFAICT, the only time these two types of terminate are different is when the
debbugger objects to the kill command. 


Øyvind
Comment 22 Alain Magloire CLA 2004-11-24 20:19:17 EST
> I didn't realize that there exists two different types of Terminate. 

Yes, actually even tree 8-)
- Selecting the Debug Target
  The kill is sent to terminate the inferior, but necessarly terminate
   the session.  Since you may have more then one target per session
  for example if gdb supports child forking.  CDT takes the action
  of terminating the launch(GDB session) if there is no other processes
  left.

- selecting the Debug session
  CDT will go and try to kill all the Debug Target/process
  whether it was succesfull or not the session is closed.

> AFAICT, the only time these two types of terminate are different is when the
> debbugger objects to the kill command. 

In the first case since the Debug Target was not sucessfully kill
the Session is still alive, remember that CDT Debug will only kill
the session for that case if all the targets are gone.

But I'm tired of fighting with buggy GDB, so your patch is accepted
the kill is sent any error/exception is ignore, the target will be
consider terminated whether it actually destroy or not.

This may not clear all of your problems but it should help.

For your other problems(Something about the build and not being
able to remove the executable), please use another PR, this one is
getting mixed with different issues ...

Flipping to fixed