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

uv run without --script forks thousands of processes, then fails, on file without .py extension #11220

Closed
ssanderson opened this issue Feb 4, 2025 · 12 comments · Fixed by #11386
Labels
bug Something isn't working

Comments

@ssanderson
Copy link
Contributor

ssanderson commented Feb 4, 2025

Summary

One of the most useful features of uv, for me, is the ability to define small utility scripts packaged alongside their dependencies. This allows me to easily use Python for many of the things I would have previously used bash for. This becomes particularly nice when using uv as your shebang line.

I recently did this on a small helper script which did not have a .py extension and I forgot to include --script. The following reproduces the scenario:

cat <<EOF > ./repro
#!/usr/bin/env -S uv run

# /// script
# dependencies = []
# ///

print("Hello world")
EOF

./repro

Running this input causes uv to hang for a while, consuming a large amount of memory (around 8GB), before eventually failing:

$ ./repro.sh
error: Failed to spawn: `./repro`
  Caused by: Argument list too long (os error 7)

Looking at the processes on my machine during the hang, it appears that uv is spawning hundreds to thousands of subprocesses:

$ ps -eaf | grep uv
ssander+  473650  473648  0 11:55 pts/1    00:00:00 uv run ./repro
ssander+  473652  473650  0 11:55 pts/1    00:00:00 uv run ./repro
ssander+  473654  473652  0 11:55 pts/1    00:00:00 uv run ./repro
ssander+  473656  473654  0 11:55 pts/1    00:00:00 uv run ./repro
ssander+  473658  473656  0 11:55 pts/1    00:00:00 uv run ./repro
ssander+  473660  473658  0 11:55 pts/1    00:00:00 uv run ./repro
ssander+  473662  473660  0 11:55 pts/1    00:00:00 uv run ./repro
ssander+  473664  473662  0 11:55 pts/1    00:00:00 uv run ./repro
ssander+  473666  473664  0 11:55 pts/1    00:00:00 uv run ./repro
ssander+  473668  473666  0 11:55 pts/1    00:00:00 uv run ./repro
ssander+  473670  473668  0 11:55 pts/1    00:00:00 uv run ./repro
ssander+  473672  473670  0 11:55 pts/1    00:00:00 uv run ./repro
// ... many more lines omitted

I assume there's some kind of recursion happening here which is effectively causing uv to forkbomb my machine.

The issue described here goes away if I add --script to the shebang line of my script, or if I rename the file to repro.py.


I don't necessarily expect uv to support the way I've invoked it here. It seems reasonable to me that uv run requires the --script flag when running a file without a .py extension, but it would be nice if it handled this class of user error a bit more gracefully.

Platform

Ubuntu 24.04, Linux 6.8.0-52-generic x86_64 GNU/Linux

Version

uv 0.5.27

Python version

Python 3.12.3

@ssanderson ssanderson added the bug Something isn't working label Feb 4, 2025
@charliermarsh
Copy link
Member

Thanks! I believe this is a duplicate of #6360.

@ssanderson
Copy link
Contributor Author

ssanderson commented Feb 4, 2025

@charliermarsh yeah, this is the same failure mode. I guess the additional question or feature request here is "is there a way to make uv handle this class of pebkac more gracefully?" As noted above, I think it's reasonable for uv to require --script in this context, but it also seems like an easy mistake for a user to leave that off, and getting fork-bombed seems like an unfortunate consequence. I realize this might be challenging to solve given the way uv run works though.

@ssanderson
Copy link
Contributor Author

I think the two ways I could imagine trying to "solve" this would be:

  1. Adding some form of dynamic recursion detection to uv run to detect if uv is running as a deeply-nested child of itself.
  2. Trying to read the target file to see if it's a text file with a shebang line starting with: /usr/bin/env -S uv run without -s or --script.

Both of these feel a bit kludgy to me, but I think both could probably be made to work well enough to catch most reasonable ways that a user might make these mistakes. I'd be happy to take a look at implementing one or the other of these as options if there's interest.

@charliermarsh
Copy link
Member

Yeah I think it makes sense to try and do a bit better here... There have been a few other requests too that would benefit from somehow having access to the command we're running.

@ssanderson
Copy link
Contributor Author

@charliermarsh would you be interested in seeing a PR toward either of the potential fixes outlined above? Without having dug too much into the details, I think one concrete proposal toward (1) might be something like:

Whenever uv run spawns a child process, check if the current process has an environment variable named something like UV_RUN_RECURSION_DEPTH set. If not set, spawn the subprocess with UV_RUN_RECURSION_DEPTH=1. If set, attempt to parse the current depth as an int, error if greater than some threshold (e.g.,10), and otherwise spawn a subprocess with the recursion depth counter incremented.

I think this would be a fairly robust and low-overhead way to catch the recursion issue here without needing to know more semantic information about the file being executed. But there are certainly other reasonable options.

@charliermarsh
Copy link
Member

Yeah it's similar to #8775. The other request I received recently was a way to tell what the current uv run command was within the executed process. I'd like to solve that problem too...

@zanieb -- thoughts on UV_RUN_RECURSION_DEPTH and UV_RUN_COMMAND being propagated to the child process?

@zanieb
Copy link
Member

zanieb commented Feb 4, 2025

I'm fine with that. Random thoughts...

  • Is there any security risk to propagating UV_RUN_COMMAND to children? Could you accidentally leak something?
  • We probably want to set UV=<self-path> whenever we spawn a child
  • Do we want to namespace the recursion tracking variable so it doesn't look user-facing? Like UV__RUN_DEPTH?
  • Is the recursion limit configurable?

@charliermarsh
Copy link
Member

Is there any security risk to propagating UV_RUN_COMMAND to children? Could you accidentally leak something?

Possibly... You already have access to sys.argv (i.e., any arguments to the script itself), and I believe we propagate all the environment variables. So the only risk would be in command-line arguments, e.g., you could have credentials in an index URL?

@zanieb
Copy link
Member

zanieb commented Feb 4, 2025

For UV_RUN_COMMAND in particular, I'd like to get a sense for the use-case. I think UV=<path> is simple enough to add without controversy.

@charliermarsh
Copy link
Member

UV__RUN_DEPTH seems relatively uncontroversial, though determining the correct limit is... unclear. It'd be nice if we could show this as a hint on failure, but the depth could be enormous.

@zanieb
Copy link
Member

zanieb commented Feb 4, 2025

I think a default UV_RUN_RECURSION_LIMIT of like.. 100 seems reasonable? 1000?

ssanderson added a commit to ssanderson/uv that referenced this issue Feb 8, 2025
ssanderson added a commit to ssanderson/uv that referenced this issue Feb 8, 2025
Handle potential infinite recursion if `uv run` recursively invokes `uv
run`. This can happen if the shebang line of a script includes `uv run`.

Handled by adding a new environment variable `UV_RUN_RECURSION_DEPTH`, which
contains a counter of the number of times that uv run has been transitively
invoked. If unset, it defaults to zero, and each time uv run starts a
subprocess it increments the counter.

Closes astral-sh#11220.
ssanderson added a commit to ssanderson/uv that referenced this issue Feb 10, 2025
Handle potential infinite recursion if `uv run` recursively invokes `uv
run`. This can happen if the shebang line of a script includes `uv run`.

Handled by adding a new environment variable `UV_RUN_RECURSION_DEPTH`, which
contains a counter of the number of times that uv run has been transitively
invoked. If unset, it defaults to zero, and each time uv run starts a
subprocess it increments the counter.

Closes astral-sh#11220.
ssanderson added a commit to ssanderson/uv that referenced this issue Feb 10, 2025
Handle potential infinite recursion if `uv run` recursively invokes `uv
run`. This can happen if the shebang line of a script includes `uv run`.

Handled by adding a new environment variable `UV_RUN_RECURSION_DEPTH`, which
contains a counter of the number of times that uv run has been transitively
invoked. If unset, it defaults to zero, and each time uv run starts a
subprocess it increments the counter.

Closes astral-sh#11220.
ssanderson added a commit to ssanderson/uv that referenced this issue Feb 10, 2025
Handle potential infinite recursion if `uv run` recursively invokes `uv
run`. This can happen if the shebang line of a script includes `uv run`.

Handled by adding a new environment variable `UV_RUN_RECURSION_DEPTH`, which
contains a counter of the number of times that uv run has been transitively
invoked. If unset, it defaults to zero, and each time uv run starts a
subprocess it increments the counter.

Closes astral-sh#11220.
ssanderson added a commit to ssanderson/uv that referenced this issue Feb 10, 2025
Handle potential infinite recursion if `uv run` recursively invokes `uv
run`. This can happen if the shebang line of a script includes `uv run`.

Handled by adding a new environment variable `UV_RUN_RECURSION_DEPTH`, which
contains a counter of the number of times that uv run has been transitively
invoked. If unset, it defaults to zero, and each time uv run starts a
subprocess it increments the counter.

Closes astral-sh#11220.
ssanderson added a commit to ssanderson/uv that referenced this issue Feb 10, 2025
Handle potential infinite recursion if `uv run` recursively invokes `uv
run`. This can happen if the shebang line of a script includes `uv run`.

Handled by adding a new environment variable `UV_RUN_RECURSION_DEPTH`, which
contains a counter of the number of times that uv run has been transitively
invoked. If unset, it defaults to zero, and each time uv run starts a
subprocess it increments the counter.

Closes astral-sh#11220.
@ssanderson
Copy link
Contributor Author

@zanieb @charliermarsh: PR with an initial crack at the infinite recursion detection here: #11386

ssanderson added a commit to ssanderson/uv that referenced this issue Feb 10, 2025
Handle potential infinite recursion if `uv run` recursively invokes `uv
run`. This can happen if the shebang line of a script includes `uv run`.

Handled by adding a new environment variable `UV_RUN_RECURSION_DEPTH`, which
contains a counter of the number of times that uv run has been transitively
invoked. If unset, it defaults to zero, and each time uv run starts a
subprocess it increments the counter.

Closes astral-sh#11220.
ssanderson added a commit to ssanderson/uv that referenced this issue Feb 10, 2025
Handle potential infinite recursion if `uv run` recursively invokes `uv
run`. This can happen if the shebang line of a script includes `uv run`.

Handled by adding a new environment variable `UV_RUN_RECURSION_DEPTH`, which
contains a counter of the number of times that uv run has been transitively
invoked. If unset, it defaults to zero, and each time uv run starts a
subprocess it increments the counter.

Closes astral-sh#11220.
ssanderson added a commit to ssanderson/uv that referenced this issue Feb 10, 2025
Handle potential infinite recursion if `uv run` recursively invokes `uv
run`. This can happen if the shebang line of a script includes `uv run` without
passing --script.

Handled by adding a new environment variable, `UV_RUN_RECURSION_DEPTH`, which
contains a counter of the number of times that uv run has been recursively
invoked. If unset, it defaults to zero, and each time uv run starts a
subprocess it increments the counter.

Closes astral-sh#11220.
ssanderson added a commit to ssanderson/uv that referenced this issue Feb 10, 2025
Handle potential infinite recursion if `uv run` recursively invokes `uv
run`. This can happen if the shebang line of a script includes `uv run` without
passing --script.

Handled by adding a new environment variable, `UV_RUN_RECURSION_DEPTH`, which
contains a counter of the number of times that uv run has been recursively
invoked. If unset, it defaults to zero, and each time uv run starts a
subprocess it increments the counter.

Closes astral-sh#11220.
ssanderson added a commit to ssanderson/uv that referenced this issue Feb 10, 2025
Handle potential infinite recursion if `uv run` recursively invokes `uv
run`. This can happen if the shebang line of a script includes `uv run` without
passing --script.

Handled by adding a new environment variable, `UV_RUN_RECURSION_DEPTH`, which
contains a counter of the number of times that uv run has been recursively
invoked. If unset, it defaults to zero, and each time uv run starts a
subprocess it increments the counter.

Closes astral-sh#11220.
@zanieb zanieb closed this as completed in 7154800 Feb 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants