Skip to content

fix: add a breaker to avoid infinite loops from source root cycles #17412

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

Merged
merged 1 commit into from
Jun 13, 2024

Conversation

davidhewitt
Copy link
Contributor

@davidhewitt davidhewitt commented Jun 13, 2024

See #17409

This patch prevents infinite looping from cycles by giving up if the number of source roots checked for a config value reaches the total number of source roots.

Alternative more precise options include creating a set of all source roots visited and giving up as soon as a cycle is encountered, but I wasn't sure how costly an allocation would be here for performance.

Can confirm that locally this fixes the problem for me.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 13, 2024
@davidhewitt davidhewitt changed the title add a breaker to avoid infinite loops from source root cycles fix: add a breaker to avoid infinite loops from source root cycles Jun 13, 2024
@Veykril
Copy link
Member

Veykril commented Jun 13, 2024

Certainly not the way to fix this but it should do for now
@bors r+

@bors
Copy link
Contributor

bors commented Jun 13, 2024

📌 Commit 0cbf900 has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jun 13, 2024

⌛ Testing commit 0cbf900 with merge 57e990b...

@davidhewitt
Copy link
Contributor Author

Thanks; completely agree that this just hides the problem at the datastructure level but at least it'll save a few watts of energy on users' machines 😂

It also doesn't seem the worst thing to have a way to break out of the infinite loop as an emergency valve. 👀

@bors
Copy link
Contributor

bors commented Jun 13, 2024

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing 57e990b to master...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants