Output of short-lived tasks is not shown

[simark]

When executing a task that ends quickly, like:

{
    "version": "2.0.0",
    "tasks": [
        {
            "label": "A task",
            "type": "shell",
            "command": "echo",
            "args": ["foo"]
        }
    ]
}

The task ends quickly, before the terminal widget has time to initialize in the front-end. The results is that the terminal widget fall back on starting the default shell. This appears in the error console:

ERROR Error attaching to terminal id 11, the terminal is most likely gone. Starting up a new terminal instead.

cc @marechal-p who reported this to me

[paul-marechal]

Because the process is too quick to execute, Theia tries to open a terminal when the process has long finished... Either store the output in some buffer to display post-mortem, or don't start a new terminal?

Why would a missing task (which should do some automated computing) end up opening a new interactive shell? Should the user finish the task by hand?

[simark]

Because the process is too quick to execute, Theia tries to open a terminal when the process has long finished... Either store the output in some buffer to display post-mortem, or don't start a new terminal?

Or wait until the terminal is ready before spawning the task.

Why would a missing task (which should do some automated computing) end up opening a new interactive shell? Should the user finish the task by hand?

I think the code that pops up a new terminal is not really meant for tasks, but for interactive terminals. If the frontend tries to restore a shell terminal that doesn't exist anymore in the backend, a new shell will be started. Since the same code is used for tasks and shell terminals, we get this weird behavior.

I also think it makes sense in the context of shell terminals, but not in the context of tasks.

[paul-marechal]

Or wait until the terminal is ready before spawning the task

+1, whatever works best, this might be easier.

I also think it makes sense in the context of shell terminals, but not in the context of tasks.

+1, here we are in a context of tasks

[vince-fugnitto]

@elaihau is this something you'd like to take a look at?

[elaihau]

@vince-fugnitto I don't have a clear picture of what could be done to fix this. Let's talk about it in detail next Monday (Aug 5) in office. would it work for you ?

[vince-fugnitto]

@elaihau yes that sounds fine, I believe even @marechal-p suggested he might work on it since I believe the bug resides in our processes.

[akosyakov]

@vince-fugnitto yes, it would be good if someone looks into it, it makes tasks quite useless, if one does not see an output

[rayakoren]

Hi, I have encountered the same issue, when the process is quick I can't see the output in the terminal.
Before diving into Theia's code I wanted to know if someone is working to fix this issue? or knows if or when it will be fixed?

[paul-marechal]

It is somewhere in my backlog indeed, but while we are at it, I would appreciate input on what I was thinking about doing:

The core of the issue is that when a process exits/closes, we dispose everything related to it. This is an issue when the execution is fast enough that the frontend cannot request for output quick enough, since it is being discarded as fast as possible.

I was thinking about storing processes and/or output in some "waiting" list, where the output would stay as long as either the frontend comes and "collects" whatever data is required, or wait for some timeout to dispose everything if the frontend didn't care about it.

This would mimic the way the Unix kernel handles processes: When a process dies, it stays into the process table until its parent calls waitpid(). Until the parent process makes that call, the child is listed as "zombie" or "defunct", and will eventually get collected by the init process if the parent exits and forgets to call waitpid.

@rayakoren @akosyakov @elaihau any opinions on that approach?

If this approach is good enough for everyone, then anyone can take that up as I don't know when I will be able to properly do this.

[a1994846931931]

Do you have any updates on this? I just encountered the same issue, which looks critical to me. I wonder if I could help 🤔...

[vince-fugnitto]

Do you have any updates on this? I just encountered the same issue, which looks critical to me. I wonder if I could help 🤔...

@a1994846931931 there was some prerequisite work done to improve processes before the issue could be tackled, but it should be possible now. @marechal-p was the original author of the processes improvements, and has also shared his design idea for the issue #2961 (comment) if you'd like to take a look. I don't believe he has the bandwidth at the moment to take a look into the problem.

[akosyakov]

I think we should reconsider the process lifecycle: clients which are responsible to create process, should be responsible to unregister it or it can be unregistered if the client is gone unexpectedly. The process should not unregister itself.

It resolves the issue since the task system will know when the task process should be released without relying on timing.

[EstherPerelman]

@simark @a1994846931931 or anyone else, I tried to reproduce this bug with the same task above - but output was shown!
Can you check again?

[vince-fugnitto]

Can you check again?

@EstherPerelman I tried with a simple echo (default task) and it did not work:

{
  "version": "2.0.0",
  "tasks": [
    {
      "label": "echo",
      "type": "shell",
      "command": "echo Hello"
    }
  ]
}

[EstherPerelman]

Can you check again?

@EstherPerelman I tried with a simple echo (default task) and it did not work:

{
  "version": "2.0.0",
  "tasks": [
    {
      "label": "echo",
      "type": "shell",
      "command": "echo Hello"
    }
  ]
}

On GitPod or Locally enviroment?

[vince-fugnitto]

On GitPod or Locally enviroment?

Local.

[EstherPerelman]

@vince-fugnitto I wonder what I'm missing @danarad05 tried it too...
We just configured this task and ran it as described what the diff? might be PC performance?

[vince-fugnitto]

We just configured this task and ran it as described what the diff? might be PC performance?

The current implementation likely relies on timing (as expressed #2961 (comment)), and we should instead fix it so it respects the process lifecycle.

[simark]

If this is a timing issue (which it likely is), you should be able to artificially add some delay somewhere to make it reproduce reliably.

[EstherPerelman]

If this is a timing issue (which it likely is), you should be able to artificially add some delay somewhere to make it reproduce reliably.

Succeeded reproduce only on gitpod but not on local Windows..

[EstherPerelman]

@simark Can I remove this line ?
I'm trying resolve this issue, And by removing that line (with one more little change) I succeeded to see output of short-lived task.
I want to ensure that it doesn't harm anything else..

The user will be responsible to unregister the process as suggested here
cc @marechal-p

[simark]

@simark Can I remove this line ?
I'm trying resolve this issue, And by removing that line (with one more little change) I succeeded to see output of short-lived task.
I want to ensure that it doesn't harm anything else..

The user will be responsible to unregister the process as suggested here
cc @marechal-p

I haven't worked on Theia for a while, but from a quick glance I don't thing that would be the right fix. Perhaps it happens to work by chance, because it makes it that some listener for this event is not called. Or, because this._killed is not set (in handleOnExit). I don't really know.

But just removing this one call, and leaving the other one (where signal is set) seems strange.