Bugzilla – Full Text Bug Listing
| Summary: | [remote debug] if gdbserver fails to launch on target, launch doesn't get terminated | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Tools] CDT | Reporter: | Anna Dushistova <anna.dushistova> | ||||||||||||
| Component: | cdt-debug-dsf-gdb | Assignee: | Marc Khouzam <marc.khouzam> | ||||||||||||
| Status: | RESOLVED FIXED | QA Contact: | Marc Khouzam <marc.khouzam> | ||||||||||||
| Severity: | normal | ||||||||||||||
| Priority: | P3 | CC: | cdtdoug, nobody, pawel.1.piech | ||||||||||||
| Version: | 8.0 | ||||||||||||||
| Target Milestone: | 8.1.0 | ||||||||||||||
| Hardware: | PC | ||||||||||||||
| OS: | Linux | ||||||||||||||
| Whiteboard: | |||||||||||||||
| Bug Depends on: | 318051 | ||||||||||||||
| Bug Blocks: | |||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Anna Dushistova
After some investigation I think that bug 318051 and this one have the same root cause. The problem is that an IProcess for the remote shell output is created separately from the debug session and thus has no control over it. There is no API(as far as I know) that allows adding a listener to the IProcess to track its termination and then kill a debug session/launch, we can only create a monitoring thread which I would rather avoid. Does anyone has any idea about how to fix this issue properly? (In reply to comment #1) > After some investigation I think that bug 318051 and this one have the same > root cause. The problem is that an IProcess for the remote shell output is > created separately from the debug session and thus has no control over it. > There is no API(as far as I know) that allows adding a listener to the IProcess > to track its termination and then kill a debug session/launch, we can only > create a monitoring thread which I would rather avoid. Does anyone has any idea > about how to fix this issue properly? The java.lang.Process you add to the launch for the remote shell should override java.lang.Process.destroy() which could then cleanup the debug session (e.g., call IGDBControl.terminate()). You can look at MIInferiorProcess.destroy() for an example. Created attachment 209947 [details]
Draft implementation of Marc's suggestion
(In reply to comment #2) > The java.lang.Process you add to the launch for the remote shell > should override java.lang.Process.destroy() which could then > cleanup the debug session (e.g., call IGDBControl.terminate()). > > You can look at MIInferiorProcess.destroy() for an example. Marc, I've attached a patch that contains a couple of my attempts. Both uncommented and commented code fix bug 318051, but still do not help with this one-session goes on initializing and then fails to connect to gdbserver. Am I missing anything here? (In reply to comment #4) > Marc, I've attached a patch that contains a couple of my attempts. > Both uncommented and commented code fix bug 318051, > this one-session goes on initializing and then fails to connect to gdbserver. > Am I missing anything here? The uncommented fix looks good for bug 318051. But it only solves the situation where the user presses the terminate button with the Remote Shell selected. It does not handle the case where the Remote Shell simply dies. I believe there are two cases to deal with for that. 1- if the Remote shell exits (because gdbserver is not found) 2- if the Remote shell dies during the debug session e.g., if you manually kill the Remote shell (kill -9), or if gdbserver crashes during the debug session. In both those cases, the new destroy() method is not called. Looking at GDBBackend.java we can see how we handle these cases for the GDB process. For #2, we use a monitoring job (GDBBackend.MonitorJob). For #1, say the path to GDB is wrong (just like your case where the path to gdbserver is wrong), we actually wait to start GDB, and once we know it is started we continue with the launch. If you look at GDBBackend.GDBProcessStep you will notice that startGdbJob only completes the RequestMonitor once we have actually communicated with GDB (received the "(gdb)" prompt). In your case, we currently don't wait to see if either gdbserver will start, or if the Remote Shell will stay alive; instead, we immediately continue with the launch (super.launch()). I'm not sure if a monitoring thread for #1 could terminate the launch in this case since it is very early and 1- the GDBControl service might not be started yet 2- if the 'target remote' command has been sent, we would first have to interrupt it before sending -gdb-exit It may be simpler to wait until we know gdbserver has started, and then continue with the call to super.launch(). So I don't think there is an easy solution to this bug. But I hope my description at least give enough info on the problem to know where we stand. (In reply to comment #5) > The uncommented fix looks good for bug 318051. I added the handling of the CDI case and attached it there. > I believe there are two cases to deal with for that. > 1- if the Remote shell exits (because gdbserver is not found) > 2- if the Remote shell dies during the debug session e.g., if you manually kill > the Remote shell (kill -9), or if gdbserver crashes during the debug session. > > In both those cases, the new destroy() method is not called. > > Looking at GDBBackend.java we can see how we handle these cases for the GDB > process. > > For #2, we use a monitoring job (GDBBackend.MonitorJob). > > For #1, say the path to GDB is wrong (just like your case where the path to > gdbserver is wrong), we actually wait to start GDB, and once we know it is > started we continue with the launch. If you look at GDBBackend.GDBProcessStep > you will notice that startGdbJob only completes the RequestMonitor once we have > actually communicated with GDB (received the "(gdb)" prompt). > > In your case, we currently don't wait to see if either gdbserver will start, or > if the Remote Shell will stay alive; instead, we immediately continue with the > launch (super.launch()). > > I'm not sure if a monitoring thread for #1 could terminate the launch in this > case since it is very early and > 1- the GDBControl service might not be started yet > 2- if the 'target remote' command has been sent, we would first have to > interrupt it before sending -gdb-exit > > It may be simpler to wait until we know gdbserver has started, and then > continue with the call to super.launch(). I hoped it would be possible to avoid this kind of solution. So do you think we should wait for "Listening on port" output and only then proceed with the launch? (In reply to comment #6) > > It may be simpler to wait until we know gdbserver has started, and then > > continue with the call to super.launch(). > > I hoped it would be possible to avoid this kind of solution. > So do you think we should wait for "Listening on port" output and only then > proceed with the launch? That sounds like a relatively easy thing to do as long as you have access to the stream that gets the output. I don't have a better idea. Created attachment 210082 [details]
Draft implementation of "wait till gdbserver started or died" idea
Marc, here is an attempt of implementation. If that will do, I'll add it to the CDI case and post a finalized patch.
(In reply to comment #8) > Created attachment 210082 [details] > Draft implementation of "wait till gdbserver started or died" idea > > Marc, here is an attempt of implementation. If that will do, I'll add it to the > CDI case and post a finalized patch. Looks good to me, except that I think you need an abort() in the case of if (monitor.isCanceled() || iProcess.isTerminated()) Created attachment 210185 [details]
Proposed fix
Marc, here is the complete patch.
However, I found an issue in the DSF case-if you close eclipse after the failed debug session, it stays alive with one of the DSF threads being active. If you do "remove all terminated" before closing, everything is fine and eclipse exits properly. CDI case doesn't have such an issue-it works as expected.
Created attachment 210201 [details] Extra fixes (In reply to comment #10) > Created attachment 210185 [details] > Proposed fix > > Marc, here is the complete patch. Thanks Anna. 1- Did you mean to use abort() instead of RSEHelper.abort(). The rest of the code uses the latter. 2- Also, try launching a valid remote session followed by a missing gdbserver one. This does not work because the RemoteGdbLaunchDelegate is only created once, so the fGdbserverReady global retains its value between session. We simply need to call setGdbserverReady(false); in the launch() method. 2.5- This second problem is actually even worse. Because the launch delegate is shared between remote sessions, it is possible that if the user starts two sessions very close together, the global variable will be used by both sessions and give the wrong result (say one launch has the right gdbserver and the other does not). I was able to trigger this problem pretty easily once I thought about it. So, we can't use a global variable. Instead we can use a final array. Unless you have a better idea? I don't :( 3- Also, one time I saw: java.lang.NullPointerException at org.eclipse.cdt.dsf.gdb.launching.ShutdownSequence.getServices(ShutdownSequence.java:87) at org.eclipse.cdt.dsf.gdb.launching.ShutdownSequence.createSteps(ShutdownSequence.java:74) at org.eclipse.cdt.dsf.gdb.launching.ShutdownSequence.<init>(ShutdownSequence.java:65) at org.eclipse.cdt.dsf.gdb.launching.GdbLaunch.shutdownSession(GdbLaunch.java:244) at org.eclipse.cdt.dsf.gdb.internal.GdbPlugin$1.execute(GdbPlugin.java:109) at org.eclipse.cdt.dsf.concurrent.Query.run(Query.java:181) at org.eclipse.cdt.dsf.concurrent.DefaultDsfExecutor$TracingWrapperRunnable.run(DefaultDsfExecutor.java:374) This is a bug in the ShutdownSequence class where we can access a null. The attached patch fixes all points and works well for me. Can you confirm it works for you and I'll commit your patch and then mine on top of yours. > However, I found an issue in the DSF case-if you close eclipse after the failed > debug session, it stays alive with one of the DSF threads being active. If you > do "remove all terminated" before closing, everything is fine and eclipse exits > properly. CDI case doesn't have such an issue-it works as expected. Can you tell me in detail how to reproduce this, I couldn't do it. (In reply to comment #11) > Created attachment 210201 [details] > Extra fixes > > (In reply to comment #10) > > Created attachment 210185 [details] > > Proposed fix > > > > Marc, here is the complete patch. > > Thanks Anna. > > 1- Did you mean to use abort() instead of RSEHelper.abort(). The rest of the > code uses the latter. I did, thanks! > 2- Also, try launching a valid remote session followed by a missing gdbserver > one. This does not work because the RemoteGdbLaunchDelegate is only created > once, so the fGdbserverReady global retains its value between session. We > simply need to call setGdbserverReady(false); in the launch() method. > > 2.5- This second problem is actually even worse. Because the launch delegate > is shared between remote sessions, it is possible that if the user starts two > sessions very close together, the global variable will be used by both sessions > and give the wrong result (say one launch has the right gdbserver and the other > does not). I was able to trigger this problem pretty easily once I thought > about it. > > So, we can't use a global variable. Instead we can use a final array. Unless > you have a better idea? I don't :( I don't either. :( > 3- Also, one time I saw: > > java.lang.NullPointerException > at > org.eclipse.cdt.dsf.gdb.launching.ShutdownSequence.getServices(ShutdownSequence.java:87) > at > org.eclipse.cdt.dsf.gdb.launching.ShutdownSequence.createSteps(ShutdownSequence.java:74) > at > org.eclipse.cdt.dsf.gdb.launching.ShutdownSequence.<init>(ShutdownSequence.java:65) > at > org.eclipse.cdt.dsf.gdb.launching.GdbLaunch.shutdownSession(GdbLaunch.java:244) > at org.eclipse.cdt.dsf.gdb.internal.GdbPlugin$1.execute(GdbPlugin.java:109) > at org.eclipse.cdt.dsf.concurrent.Query.run(Query.java:181) > at > org.eclipse.cdt.dsf.concurrent.DefaultDsfExecutor$TracingWrapperRunnable.run(DefaultDsfExecutor.java:374) > > This is a bug in the ShutdownSequence class where we can access a null. > > The attached patch fixes all points and works well for me. > Can you confirm it works for you and I'll commit your patch and then mine on > top of yours. Works fine for me too, thanks for helping me out! > > However, I found an issue in the DSF case-if you close eclipse after the failed > > debug session, it stays alive with one of the DSF threads being active. If you > > do "remove all terminated" before closing, everything is fine and eclipse exits > > properly. CDI case doesn't have such an issue-it works as expected. > > Can you tell me in detail how to reproduce this, I couldn't do it. 1) Create a DSF remote(I am using "Linux" connection type with everything ssh) launch configuration with an incorrect gdbserver name(I am using gdbserver1); 2) Launch it; 3) See the error dialog we've just added; 4) Close the dialog, close eclipse-I can still see it in my debug view as running. Here is the stack: New_configuration [Eclipse Application] org.eclipse.equinox.launcher.Main at localhost:33249 Thread [main] (Running) Thread [Framework Active Thread] (Running) Daemon Thread [State Data Manager] (Running) Daemon Thread [Start Level Event Dispatcher] (Running) Daemon Thread [Framework Event Dispatcher] (Running) Daemon Thread [[Timer] - Main Queue Handler] (Running) Thread [Worker-JM] (Running) Thread [Worker-0] (Running) Thread [Worker-1] (Running) Thread [Worker-3] (Running) Thread [Worker-4] (Running) Thread [Worker-5] (Running) Thread [Worker-6] (Running) Thread [Worker-7] (Running) Thread [Worker-8] (Running) Thread [Worker-9] (Running) Thread [Worker-10] (Running) Thread [Worker-11] (Running) Thread [Worker-12] (Running) Daemon Thread [[ThreadPool Manager] - Idle Thread] (Running) Daemon Thread [Java indexing] (Running) Thread [org.eclipse.cdt.dsf.gdb - 0] (Running) Thread [Connect thread LOCALHOST session] (Running) Thread [Terminal Service ShellWriterThread-10] (Running) /usr/lib/jvm/java-1.6.0-sun-1.6.0/bin/java (Jan 27, 2012 8:20:48 PM) I am using Indigo, I am on openSuse 11.2, java 1.6. Do you want me to try Juno and see if I am still able to reproduce it? (In reply to comment #12) > > 1- Did you mean to use abort() instead of RSEHelper.abort(). The rest of the > > code uses the latter. > > I did, thanks! I'm not sure what you really mean :) Should I replace abort() with RSEHelper.abort()? > > The attached patch fixes all points and works well for me. > > Can you confirm it works for you and I'll commit your patch and then mine on > > top of yours. > > Works fine for me too, thanks for helping me out! No problem. I will commit them. > > > However, I found an issue in the DSF case-if you close eclipse after the failed > > > debug session, it stays alive with one of the DSF threads being active. If you > > > do "remove all terminated" before closing, everything is fine and eclipse exits > > > properly. CDI case doesn't have such an issue-it works as expected. > > > > Can you tell me in detail how to reproduce this, I couldn't do it. > > 1) Create a DSF remote(I am using "Linux" connection type with everything ssh) > launch configuration with an incorrect gdbserver name(I am using gdbserver1); > 2) Launch it; > 3) See the error dialog we've just added; > 4) Close the dialog, close eclipse-I can still see it in my debug view as > running. Here is the stack: > > New_configuration [Eclipse Application] > org.eclipse.equinox.launcher.Main at localhost:33249 > Thread [main] (Running) > Thread [Framework Active Thread] (Running) > Daemon Thread [State Data Manager] (Running) > Daemon Thread [Start Level Event Dispatcher] (Running) > Daemon Thread [Framework Event Dispatcher] (Running) > Daemon Thread [[Timer] - Main Queue Handler] (Running) > Thread [Worker-JM] (Running) > Thread [Worker-0] (Running) > Thread [Worker-1] (Running) > Thread [Worker-3] (Running) > Thread [Worker-4] (Running) > Thread [Worker-5] (Running) > Thread [Worker-6] (Running) > Thread [Worker-7] (Running) > Thread [Worker-8] (Running) > Thread [Worker-9] (Running) > Thread [Worker-10] (Running) > Thread [Worker-11] (Running) > Thread [Worker-12] (Running) > Daemon Thread [[ThreadPool Manager] - Idle Thread] (Running) > Daemon Thread [Java indexing] (Running) > Thread [org.eclipse.cdt.dsf.gdb - 0] (Running) > Thread [Connect thread LOCALHOST session] (Running) > Thread [Terminal Service ShellWriterThread-10] (Running) > /usr/lib/jvm/java-1.6.0-sun-1.6.0/bin/java (Jan 27, 2012 8:20:48 PM) > > > I am using Indigo, I am on openSuse 11.2, java 1.6. Do you want me to try Juno > and see if I am still able to reproduce it? I'll try your steps and let you know. Ok, I can reproduce the problem. I think it is because of this NPE. I'll ook into it. java.lang.NullPointerException at org.eclipse.cdt.dsf.gdb.launching.GdbLaunch$4.handleCompleted(GdbLaunch.java:260) at org.eclipse.cdt.dsf.concurrent.RequestMonitor$2.run(RequestMonitor.java:303) at org.eclipse.cdt.dsf.concurrent.DefaultDsfExecutor$TracingWrapperRunnable.run(DefaultDsfExecutor.java:374) at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:441) at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:303) at java.util.concurrent.FutureTask.run(FutureTask.java:138) at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:98) at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:206) at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:886) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:908) at java.lang.Thread.run(Thread.java:662) Twice now, right after launching eclipse, when I do a valid remote launch I see this exception. Anna, this is in RSE stuff, can you have a look? java.lang.IllegalThreadStateException at java.lang.Thread.start(Thread.java:638) at org.eclipse.rse.services.shells.AbstractHostShellOutputReader.startIfNotAlive(AbstractHostShellOutputReader.java:111) at org.eclipse.rse.services.shells.AbstractHostShellOutputReader.addOutputListener(AbstractHostShellOutputReader.java:139) at org.eclipse.rse.services.shells.HostShellProcessAdapter.<init>(HostShellProcessAdapter.java:59) at org.eclipse.cdt.launch.remote.launching.RemoteGdbLaunchDelegate$2.<init>(RemoteGdbLaunchDelegate.java:135) at org.eclipse.cdt.launch.remote.launching.RemoteGdbLaunchDelegate.launch(RemoteGdbLaunchDelegate.java:135) at org.eclipse.debug.internal.core.LaunchConfiguration.launch(LaunchConfiguration.java:855) at org.eclipse.debug.internal.core.LaunchConfiguration.launch(LaunchConfiguration.java:704) at org.eclipse.debug.internal.ui.DebugUIPlugin.buildAndLaunch(DebugUIPlugin.java:951) at org.eclipse.debug.internal.ui.DebugUIPlugin$8.run(DebugUIPlugin.java:1155) at org.eclipse.core.internal.jobs.Worker.run(Worker.java:54) I'll see what's going on there. (In reply to comment #15) > Twice now, right after launching eclipse, when I do a valid remote launch I see > this exception. > > Anna, this is in RSE stuff, can you have a look? > > java.lang.IllegalThreadStateException > at java.lang.Thread.start(Thread.java:638) > at > org.eclipse.rse.services.shells.AbstractHostShellOutputReader.startIfNotAlive(AbstractHostShellOutputReader.java:111) > at > org.eclipse.rse.services.shells.AbstractHostShellOutputReader.addOutputListener(AbstractHostShellOutputReader.java:139) > at > org.eclipse.rse.services.shells.HostShellProcessAdapter.<init>(HostShellProcessAdapter.java:59) > at > org.eclipse.cdt.launch.remote.launching.RemoteGdbLaunchDelegate$2.<init>(RemoteGdbLaunchDelegate.java:135) > at > org.eclipse.cdt.launch.remote.launching.RemoteGdbLaunchDelegate.launch(RemoteGdbLaunchDelegate.java:135) > at > org.eclipse.debug.internal.core.LaunchConfiguration.launch(LaunchConfiguration.java:855) > at > org.eclipse.debug.internal.core.LaunchConfiguration.launch(LaunchConfiguration.java:704) > at > org.eclipse.debug.internal.ui.DebugUIPlugin.buildAndLaunch(DebugUIPlugin.java:951) > at > org.eclipse.debug.internal.ui.DebugUIPlugin$8.run(DebugUIPlugin.java:1155) > at org.eclipse.core.internal.jobs.Worker.run(Worker.java:54) Created attachment 210212 [details] Fix for NPE in GdbLaunch (In reply to comment #14) > java.lang.NullPointerException > at > org.eclipse.cdt.dsf.gdb.launching.GdbLaunch$4.handleCompleted(GdbLaunch.java:260) Here is a patch that fixes the problem. What is happening is the following. When we notice that gdbserver has failed to start, we call remoteShellProcess.destroy() which tries to call IGDBControl.terminate(). However, in this case, the DSF services have not been started because we are still in the middle of the launch, so terminate() is not called. On the other hand, the GdbLaunch has already been created which has create the DSF session. So, we still need to shutdown that session using GdbLaunch.shutdownSession(). I added this logic. Finally, when calling GdbLaunch.shutdownSession() from a partial launch, we ended up with an NPE because the GdbLaunch.fMemRetrieval variable had not been created yet. So, I added a guard against a null in this case. This patch fixes this situation. I've committed it to master. Anna, can you test it please, and make sure that exiting Eclipse now works properly for you? Thanks (In reply to comment #17) > Created attachment 210212 [details] > This patch fixes this situation. I've committed it to master. > > Anna, can you test it please, and make sure that exiting Eclipse now works > properly for you? It does, thanks a lot! I wasn't able to reproduce the RSE exception you had yet. *** cdt git genie on behalf of Marc Khouzam ***
Bug 368597: [remote debug] if gdbserver fails to launch on target, launch doesn't get terminated
[*] http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=55e8d9f684deae4dcc13a28b0513bac5db7f927a
*** cdt git genie on behalf of Anna Dushistova ***
Bug 368597: [remote debug] if gdbserver fails to launch on target, launch doesn't get terminated
[*] http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=13d3bad2dec3762cc90d7380749328be51a63164
*** cdt git genie on behalf of Marc Khouzam ***
Bug 368597: Although we don't complete the launch, we still need to shutdown the DSF session as it was started already.
[*] http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=f13bdf3b334e088e06f032879ebe20f2836ca414
Marc, is there any reason it is still open? Please, reopen if so. I was still open because of the below exception. Is it no longer happening? (In reply to comment #16) > I'll see what's going on there. > > (In reply to comment #15) > > Twice now, right after launching eclipse, when I do a valid remote launch I see > > this exception. > > > > Anna, this is in RSE stuff, can you have a look? > > > > java.lang.IllegalThreadStateException > > at java.lang.Thread.start(Thread.java:638) > > at > > org.eclipse.rse.services.shells.AbstractHostShellOutputReader.startIfNotAlive(AbstractHostShellOutputReader.java:111) > > at > > org.eclipse.rse.services.shells.AbstractHostShellOutputReader.addOutputListener(AbstractHostShellOutputReader.java:139) > > at > > org.eclipse.rse.services.shells.HostShellProcessAdapter.<init>(HostShellProcessAdapter.java:59) > > at > > org.eclipse.cdt.launch.remote.launching.RemoteGdbLaunchDelegate$2.<init>(RemoteGdbLaunchDelegate.java:135) > > at > > org.eclipse.cdt.launch.remote.launching.RemoteGdbLaunchDelegate.launch(RemoteGdbLaunchDelegate.java:135) > > at > > org.eclipse.debug.internal.core.LaunchConfiguration.launch(LaunchConfiguration.java:855) > > at > > org.eclipse.debug.internal.core.LaunchConfiguration.launch(LaunchConfiguration.java:704) > > at > > org.eclipse.debug.internal.ui.DebugUIPlugin.buildAndLaunch(DebugUIPlugin.java:951) > > at > > org.eclipse.debug.internal.ui.DebugUIPlugin$8.run(DebugUIPlugin.java:1155) > > at org.eclipse.core.internal.jobs.Worker.run(Worker.java:54) I wasn't able to reproduce it. (In reply to comment #24) > I wasn't able to reproduce it. I can't either anymore. Thanks! |