Shutdown actions are not getting completed, when launch is terminated and launches it again immediately

[pnipin]

Describe the bug
When a launch is terminated and relaunch it (or any other launch) too fast, executor of previous launch will get shutdown before shutdown sequence is completed and therefore not firing the shutdown function of services.

To Reproduce
Steps to reproduce the behavior:

1. Terminate an active launch
2. Launch it again using debug button in tool bar immediately when previous launch shows as terminated in debug view.

Expected behavior
When a launch is terminated and relaunch again, terminated launch should get enough time to fire shutdown activities of all the registered services,

Version Information (please complete the following information):

• OS and OS Version/extra details: Windows 11
• Eclipse Version: 2023/09 Platform version : 4.29
• CDT Version : 11.3.1

[pnipin]

@jonahgraham , Could you please share your thoughts?

[jonahgraham]

Very odd and doesn't match the behaviour I see. Are you a CDT user or a CDT developer as the correct follow up depends on which path you are on.

CDT User

Please let me know what shutdown activities you are not seeing run. How do you know they aren't run (e.g. missing expected commands in gdb traces or hardware not left in expected state)

CDT Dev

Can you share what is terminating the executor too soon? Can you put breakpoints DefaultDsfExecutor and some service to see them getting shutdown improperly.

What I see

I can see the a message like this (with tracing on):

263,319 Executor (org.eclipse.cdt.dsf.gdb - 2) is being shut down. Already submitted tasks will be executed, new ones will not.

But that shutdown method should only be called if the launch is terminated (i.e org.eclipse.debug.core.Launch.isTerminated() returns true)

What I can see is that gdb is closed cleanly higher up in the log before the shutdown.

040,280 [MI]  41-gdb-exit
040,281 [MI]  41^exit
040,282 [MI]  =thread-exited,id="1",group-id="i1"
040,282 [MI]  =thread-group-exited,id="i1"

[pnipin]

Thank you for the quick response.
I am a CDT developer.

I can see executor is terminated from the launchRemoved function in GdbLaunch class (which is fired from a different thread):

Same executor is used for executing the shutdown sequence:

Shutdown sequence contains steps which fire shutdown() function in services. I have put breakpoint in shutdown() function of my service and can see it is not executed (which executes in normal case):
ShutdownSequence.java

[jonahgraham]

OK, I can see what is happening - GDB and other runtime processes attached to the launch are shutting down cleanly, leading to org.eclipse.debug.core.Launch.isTerminated() returning true. But then I see what you observe, a race condition because if the service hasn't finished shutting down, the executor is stopped anyway.

I think GdbLaunch (subclass of Launch) should not return true from isTerminated until the full shutdown sequence is complete in org.eclipse.cdt.dsf.gdb.launching.GdbLaunch.shutdownSession(RequestMonitor).

Here is a quickly coded change that may fix this. Can you test/modify this and provide it as a PR? I think it should probably not rely on state of fTracker and maybe a new flag that explicitly means the right thing should be created.

diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/launching/GdbLaunch.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/launching/GdbLaunch.java
index e072dd4c4d..05b72a84b8 100644
--- a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/launching/GdbLaunch.java
+++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/launching/GdbLaunch.java
@@ -345,6 +345,15 @@ public class GdbLaunch extends DsfLaunch implements ITracedLaunch, ITargetedLaun
                handler.execute(req);
        }
 
+       @Override
+       public boolean isTerminated() {
+               // This test matches the last step in shutdownSession
+               if (fTracker != null) {
+                       return false;
+               }
+               return super.isTerminated();
+       }
+
        // IDisconnect
        ///////////////////////////////////////////////////////////////////////////
 

PS We have a slight testing problem because some DSF tests are not working on GitHub actions, see #816

[pnipin]

Thanks. tested the fix.
It is giving an error pop up that previous launch is not terminated and allowing to shut down its services

It is saving the purpose. It will be good if new launch will wait for proper exit of first launch and then continue.

The thing is that debug view is showing the launch as terminated (but it is actually not), user will be going for the relaunch and seeing this pop up will be confusing.

[jonahgraham]

I can't reproduce what you are seeing in this case. Let me run down what I see and perhaps we can see where the difference is.

1. org.eclipse.debug.internal.ui.commands.actions.TerminateAndRelaunchAction is the action that handles terminate and relaunch button.
2. When button pressed, execute (in super class DebugCommandAction) calls getCommandType in TerminateAndRelaunchAction to find the command type to run, in this case org.eclipse.debug.core.commands.ITerminateHandler
3. The command type is looked up in the adapter factory to get the handler implementation, in my case org.eclipse.cdt.dsf.gdb.internal.ui.actions.DsfTerminateCommand
4. DsfTerminateCommand.execute is run. It is passed in a IDebugCommandRequest
5. All the shutdown operations are run by calling gdbControl.terminate and when that completes its handleCompleted calls done on the IDebugCommandRequest
6. The relaunch in TerminateAndRelaunchAction is done in the postExecute method, which is called by the done call in the above step.

Does that help identify the problem?

[pnipin]

Actually, Terminate and relaunch button action is working fine.
I think, I communicated the issue wrongly.
Please see the sequence below:

1. Terminated a debug session, and tried to launch it again immediately (using debug button).
2. Executor of previous launch will get shutdown before shutdown sequence is completed and therefore not firing the shutdown function of services.
3. After adding the fix, previous launch is allowed to complete the shut down sequences. And the new launch is failing with an error message saying previous launch's termination is not yet completed,
4. So user knows that some termination processes are still going on. The concern is, I tried to launch a debug session when I see the previous launch is shown terminated in debug view. But actually, termination was not over and error pops up :

So, Is there any other solution like new launch wait till the termination activities of the previous launch completes?

[jonahgraham]

comments/questions so we can progress this further:

I tried to launch a debug session when I see the previous launch is shown terminated in debug view

IIUC the executor is still running but the UI is showing as terminated? Are all nodes shown as terminated, or just the gdb one. In particular is the root shown as terminated like this:

"Terminate the first one before restarting" screenshot

This message comes from embed-cdt (line here).

I think it would be possible to add more logic to that code to actually delay the launch, or do a relaunch rather than erroring out. Please raise an issue on the embed-cdt project and tag me (I am a committer on both, but we need to involve the rest of the embed-cdt community as there may be some other inputs there)