Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Function aliases in subprocesses can cause Xonsh to exit #4577

Closed
mingmingrr opened this issue Nov 30, 2021 · 1 comment
Closed

Function aliases in subprocesses can cause Xonsh to exit #4577

mingmingrr opened this issue Nov 30, 2021 · 1 comment

Comments

@mingmingrr
Copy link

mingmingrr commented Nov 30, 2021

Any aliases that are python functions has a seemingly random chance of causing Xonsh to exit, if they are used at the ends of pipes inside subprocesses.

xonfig

$ xonfig
+------------------+-----------------+
| xonsh            | 0.11.0          |
| Python           | 3.9.6           |
| PLY              | 3.11            |
| have readline    | True            |
| prompt toolkit   | 3.0.19          |
| shell type       | readline        |
| history backend  | json            |
| pygments         | 2.10.0          |
| on posix         | True            |
| on linux         | True            |
| distro           | unknown         | (NixOs 22.04 unstable)
| on wsl           | False           |
| on darwin        | False           |
| on windows       | False           |
| on cygwin        | False           |
| on msys2         | False           |
| is superuser     | False           |
| default encoding | utf-8           |
| xonsh encoding   | utf-8           |
| encoding errors  | surrogateescape |
| on jupyter       | False           |
| jupyter kernel   | None            |
| xontrib          | []              |
| RC file          | []              |
+------------------+-----------------+

Expected Behavior

aliases['foo'] = lambda args: 0
$(echo | foo)
# should produce `''` forever

Current Behavior

Around 1/4 of the time, $(echo | foo) triggers a TypeError: an integer is required (got type NoneType) and causes Xonsh to exit.

$(foo) # works fine
$(echo | foo | cat) # works fine
echo | foo # works fine

Traceback (if applicable)

[bash] $ # shell type doesn't matter, ptk just produces an extra termios error 
[bash] $ XONSH_DEBUG=1 XONSH_SHOW_TRACEBACK=1 xonsh --no-rc --shell-type rl
mingmingrr@potato ~ master $  history files for 0.0160s.
mingmingrr@potato ~ master $ aliases['foo'] = lambda args: 0
mingmingrr@potato ~ master $ $(echo | foo)
''
mingmingrr@potato ~ master $ $(echo | foo)
''
mingmingrr@potato ~ master $ $(echo | foo)
xonsh: To log full traceback to a file set: $XONSH_TRACEBACK_LOGFILE = <filename>
Traceback (most recent call last):
  File "/nix/store/7pxlzy2j9dnz6aagg6j6lyni12f1psm8-xonsh-0.11.0/lib/python3.9/site-packages/xonsh/__amalgam__.py", line 16776, in default
    run_compiled_code(code, self.ctx, None, "single")
  File "/nix/store/7pxlzy2j9dnz6aagg6j6lyni12f1psm8-xonsh-0.11.0/lib/python3.9/site-packages/xonsh/__amalgam__.py", line 3563, in run_compiled_code
    func(code, glb, loc)
  File "<xonsh-code>", line 1, in <module>
  File "/nix/store/7pxlzy2j9dnz6aagg6j6lyni12f1psm8-xonsh-0.11.0/lib/python3.9/site-packages/xonsh/built_ins.py", line 166, in subproc_captured_stdout
    return xonsh.procs.specs.run_subproc(cmds, captured="stdout", envs=envs)
  File "/nix/store/7pxlzy2j9dnz6aagg6j6lyni12f1psm8-xonsh-0.11.0/lib/python3.9/site-packages/xonsh/procs/__amalgam__.py", line 3582, in run_subproc
    command.end()
  File "/nix/store/7pxlzy2j9dnz6aagg6j6lyni12f1psm8-xonsh-0.11.0/lib/python3.9/site-packages/xonsh/procs/__amalgam__.py", line 928, in end
    self._end(tee_output=tee_output)
  File "/nix/store/7pxlzy2j9dnz6aagg6j6lyni12f1psm8-xonsh-0.11.0/lib/python3.9/site-packages/xonsh/procs/__amalgam__.py", line 936, in _end
    for _ in self.tee_stdout():
  File "/nix/store/7pxlzy2j9dnz6aagg6j6lyni12f1psm8-xonsh-0.11.0/lib/python3.9/site-packages/xonsh/procs/__amalgam__.py", line 838, in tee_stdout
    for line in self.iterraw():
  File "/nix/store/7pxlzy2j9dnz6aagg6j6lyni12f1psm8-xonsh-0.11.0/lib/python3.9/site-packages/xonsh/procs/__amalgam__.py", line 723, in iterraw
    task = xj.wait_for_active_job()
  File "/nix/store/7pxlzy2j9dnz6aagg6j6lyni12f1psm8-xonsh-0.11.0/lib/python3.9/site-packages/xonsh/__amalgam__.py", line 11043, in wait_for_active_job
    _, wcode = os.waitpid(obj.pid, os.WUNTRACED)
TypeError: an integer is required (got type NoneType)

[1]+  Stopped                 XONSH_DEBUG=1 XONSH_SHOW_TRACEBACK=1 xonsh --no-rc
[bash] $

Steps to Reproduce

aliases['foo'] = lambda args: 0
$(echo | foo) # run a few times

Edit: trying to work around this with two aliases does not work:

aliases['_foo'] = lambda args: 0
aliases['foo'] = '_foo | cat'
$(echo | foo) # causes same error

For community

⬇️ Please click the 👍 reaction instead of leaving a +1 or 👍 comment

@anki-code
Copy link
Member

anki-code commented May 19, 2024

I can't reproduce this.

for i in range(50):
  aliases['foo'] = lambda args: 0
  $(echo | foo) # run a few times
# success

gforsyth added a commit that referenced this issue May 22, 2024
Reading stop signals from the process and update the process state.

### The issue

Technically. In a couple of places that critical for processing signals
we have `os.waitpid()`. The function behavior is pretty unobvious and
one of things is processing return code after catching the signal. We
had no good signal processing around this and this PR fixes this. See
also `proc_untraced_waitpid` function description.

From user perspective. For example we have process that is waiting for
user input from terminal e.g. `python -c "input()"` or `fzf`. If this
process will be in captured pipeline e.g. `!(echo 1 | fzf | head)` it
will be suspended by OS and the pipeline will be in the endless loop
with future crashing and corrupting std at the end. This PR fixes this.

### The solution

Technically. The key function is `proc_untraced_waitpid` - it catches
the stop signals and updates the process state.

From user perspective. First of all we expect that users will use
captured object `!()` only for capturable processes. Because of it our
goal here is to just make the behavior in this case stable.
In this PR we detect that process in the pipeline is suspended and we
need to finish the command pipeline carefully:
* Show the message about suspended process.
* Keep suspended process in `jobs`. The same behavior we can see in
bash. This is good because we don't know what process suspended and why.
May be experienced user will want to continue it manually.
* Finish the CommandPipeline with returncode=None and suspended=True.

### Before

```xsh
!(fzf) # or !(python -c "input()")
# Hanging / Exceptions / OSError / No way to end the command.
# After exception:
$(echo 1)
# OSError / IO error
```

### After

```xsh
!(fzf) # or `!(ls | fzf | head)` or `!(python -c "input()")`
# Process ['fzf'] with pid 60000 suspended with signal 22 SIGTTOU and stay in `jobs`.
# This happends when process start waiting for input but there is no terminal attached in captured mode.
# CommandPipeline(returncode=None, suspended=True, ...)

$(echo 1)
# Success.
```
Closes #4752 #4577

### Notes

* There is pretty edge case situation when the process was terminated so
fast that we can't catch pid alive and check signal
([src](https://github.com/xonsh/xonsh/blob/67d672783db6397bdec7ae44a9d9727b1e20a772/xonsh/jobs.py#L71-L80)).
I leave it as is for now.

### Mentions

#2159

## For community
⬇️ **Please click the 👍 reaction instead of leaving a `+1` or 👍
comment**

---------

Co-authored-by: a <1@1.1>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Gil Forsyth <gforsyth@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants