Detect infinite recursion in uv run. #11386

ssanderson · 2025-02-10T14:47:21Z

Summary

Handle potential infinite recursion if uv run recursively invokes uv run. This can happen if the shebang line of a script includes uv run, but does not pass --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 we increment the counter, erroring if the value is greater than a configurable (but not currently exposed or documented) threshold.

Closes #11220.

Test Plan

I've added a snapshot test to uv/crates/uv/tests/it/run that tests the end-to-end recursion detection flow. I've currently made it a unix-only test because I'm not sure offhand how uv run will interact with shebang lines on windows.

crates/uv-cli/src/lib.rs

crates/uv-static/src/env_vars.rs

ssanderson · 2025-02-10T14:50:43Z

crates/uv/src/commands/project/run.rs

 ) -> anyhow::Result<ExitStatus> {
+    // Check if max recursion depth was exceeded. This most commonly happens


This check could be pushed up higher in the call stack, but it seemed nice to have the logic for the check and the logic for setting/incrementing the counter in the same place.

ssanderson · 2025-02-10T15:08:11Z

crates/uv/src/settings.rs

+    // enough that it's unlikely a user actually needs this recursion depth,
+    // but short enough that we detect recursion quickly enough to avoid OOMing
+    // or hanging for a long time.
+    const DEFAULT_MAX_RECURSION_DEPTH: u32 = 100;


At a depth of 100 this takes about 2.5 seconds to error on a relatively beefy laptop. The time to detection scales more or less linearly with this value, so I think the way to set this is basically to pick something large enough to be unlikely to cause problems, but low enough that we actually detect recursion in enough time to be useful.

The 2.5s number above is from running in debug, so if a target error time of ~2s sounds ok, then this can probably be bumped up to 500-1000 in release.

zanieb · 2025-02-10T17:57:11Z

crates/uv-cli/src/lib.rs

+    #[arg(long, hide = true, env = EnvVars::UV_RUN_MAX_RECURSION_DEPTH)]
+    pub max_recursion_depth: Option<u32>,


I can see a case for adding this to the CLI but... we could also just read it directly. Since you already did this, it seems okay to leave it (as a hidden option).

Yeah, I made this configurable primarily to support testing (without this being configurable the test for this takes 2-3 seconds in debug), but this seemed like something that a user could theoretically want to control, and it isn't too hard to plumb this to where it needs to go.

zanieb · 2025-02-10T17:58:26Z

Thanks for contributing! This looks pretty good to me.

It's fine to gate the test case to Unix.

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

I made some small changes. Thank you!

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [astral-sh/uv](https://github.com/astral-sh/uv) | patch | `0.5.30` -> `0.5.31` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>astral-sh/uv (astral-sh/uv)</summary> ### [`v0.5.31`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#0531) [Compare Source](astral-sh/uv@0.5.30...0.5.31) ##### Enhancements - Add `uv sync --script` ([#11361](astral-sh/uv#11361)) - Allow PEP 508 requirements in tool requests ([#11337](astral-sh/uv#11337)) - Allow source distributions to produce wheels with `+local` suffixes ([#11429](astral-sh/uv#11429)) - Bring parity to `uvx` and `uv tool install` requests ([#11345](astral-sh/uv#11345)) - Use a stable directory for local, remote, and stdin script virtual environments ([#11347](astral-sh/uv#11347), [#11364](astral-sh/uv#11364)) - Detect infinite recursion in `uv run` ([#11386](astral-sh/uv#11386)) ##### Python The managed Python distributions have been updated, including: - CPython 3.14.0a5, which includes a new [tail calling interpreter](https://docs.python.org/3.14/whatsnew/3.14.html#whatsnew314-tail-call) for a significant performance improvement - The bundled OpenSSL version was updated from 3.0.15 to 3.0.16 which fixes a [security advisory](https://openssl-library.org/news/secadv/20241016.txt) See the [`python-build-standalone` release notes](https://github.com/astral-sh/python-build-standalone/releases/tag/20250212) for more details. ##### Bug fixes - Fix cross-drive script installation ([#11167](astral-sh/uv#11167)) - Add indexes in priority order ([#11451](astral-sh/uv#11451)) - Allow `--python <dir>` requests to match existing environments if `sys.executable` is the same file ([#11290](astral-sh/uv#11290)) - Avoid comparing to system site packages in `--dry-run` mode ([#11427](astral-sh/uv#11427)) - Prefer running executables in the environment with `<name>` over `<name>/__main__.py` ([#11431](astral-sh/uv#11431)) - Retry local clones without hardlinks if they fail ([#11421](astral-sh/uv#11421)) ##### Documentation - Update alternative-indexes.md to use `UV_INDEX` instead of `UV_EXTRA_INDEX_URL` ([#11381](astral-sh/uv#11381)) - Update scripts guide to include using package indexes ([#11443](astral-sh/uv#11443)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

ssanderson commented Feb 10, 2025

View reviewed changes

crates/uv-cli/src/lib.rs Outdated Show resolved Hide resolved

ssanderson commented Feb 10, 2025

View reviewed changes

crates/uv-static/src/env_vars.rs Outdated Show resolved Hide resolved

ssanderson commented Feb 10, 2025

View reviewed changes

ssanderson force-pushed the check-recursion-depth branch 4 times, most recently from c8665e1 to 170c702 Compare February 10, 2025 15:05

ssanderson commented Feb 10, 2025

View reviewed changes

ssanderson mentioned this pull request Feb 10, 2025

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

Closed

ssanderson force-pushed the check-recursion-depth branch 2 times, most recently from 3fa9ccb to 2dfb202 Compare February 10, 2025 15:56

zanieb self-assigned this Feb 10, 2025

zanieb reviewed Feb 10, 2025

View reviewed changes

ssanderson force-pushed the check-recursion-depth branch 2 times, most recently from 6bf64b9 to 2d19f17 Compare February 10, 2025 19:52

ssanderson force-pushed the check-recursion-depth branch from 2d19f17 to e4c7b90 Compare February 10, 2025 19:56

Review

e7b1d82

zanieb approved these changes Feb 12, 2025

View reviewed changes

zanieb enabled auto-merge (squash) February 12, 2025 13:44

zanieb added 2 commits February 12, 2025 12:47

Lint

12bd203

Merge branch 'main' into check-recursion-depth

3718974

zanieb merged commit 7154800 into astral-sh:main Feb 12, 2025
73 checks passed

BrewTestBot mentioned this pull request Feb 12, 2025

uv 0.5.31 Homebrew/homebrew-core#207491

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Detect infinite recursion in uv run. #11386

Detect infinite recursion in uv run. #11386

ssanderson commented Feb 10, 2025 •

edited

Loading

ssanderson Feb 10, 2025

ssanderson Feb 10, 2025 •

edited

Loading

zanieb Feb 10, 2025

ssanderson Feb 10, 2025 •

edited

Loading

zanieb commented Feb 10, 2025

zanieb left a comment

		) -> anyhow::Result<ExitStatus> {
		// Check if max recursion depth was exceeded. This most commonly happens

		#[arg(long, hide = true, env = EnvVars::UV_RUN_MAX_RECURSION_DEPTH)]
		pub max_recursion_depth: Option<u32>,

Detect infinite recursion in uv run. #11386

Detect infinite recursion in uv run. #11386

Conversation

ssanderson commented Feb 10, 2025 • edited Loading

Summary

Test Plan

ssanderson Feb 10, 2025

Choose a reason for hiding this comment

ssanderson Feb 10, 2025 • edited Loading

Choose a reason for hiding this comment

zanieb Feb 10, 2025

Choose a reason for hiding this comment

ssanderson Feb 10, 2025 • edited Loading

Choose a reason for hiding this comment

zanieb commented Feb 10, 2025

zanieb left a comment

Choose a reason for hiding this comment

ssanderson commented Feb 10, 2025 •

edited

Loading

ssanderson Feb 10, 2025 •

edited

Loading

ssanderson Feb 10, 2025 •

edited

Loading