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

Avoid stack overflow inside StubSpecification on some edge cases #3635

Merged
merged 5 commits into from May 21, 2020

Conversation

deivid-rodriguez
Copy link
Member

Description:

This is a port of rubygems/bundler#6653 from the old repo, which closes #3365.

Tasks:

  • Describe the problem / feature
  • Write tests
  • Write code to solve the problem
  • Get code review from coworkers / friends

I will abide by the code of conduct.

@deivid-rodriguez deivid-rodriguez marked this pull request as draft May 17, 2020 13:26
@deivid-rodriguez deivid-rodriguez force-pushed the fix_bundle_exec_bundle_install_git_extension branch from a62b770 to c98e2f8 Compare May 18, 2020 12:46
deivid-rodriguez and others added 5 commits May 19, 2020 12:36
The higher level consequence of defining the method is correct, since
each source checks `missing_specs?`  of every installed spec when
building its index

However, the purpose of the specific method is to avoid loading the
corresponding full specification of that stub, so let's make a more
focused comment.
This method name had a typo, since the method in the full specification
is called `default_gem?` not `default_gem`. That typo being unnoticed
most likely indicates that the method was not being run at all. However,
since I intend to use it in later commits, I'm fixing the typo and not
removing it.
By explicitly defining a couple more methods, we can avoid loading the
full specification in all cases, including pathological ones that
currently lead to stackoverflow errors.
@deivid-rodriguez deivid-rodriguez force-pushed the fix_bundle_exec_bundle_install_git_extension branch from d767edc to 226ec11 Compare May 19, 2020 10:37
@deivid-rodriguez
Copy link
Member Author

I ended up changing the strategy from the original PR to make sure that we don't load the full specification even in the edge case of the referenced issue. Also added some other improvements while investigating the issue. I believe it's ready.

@deivid-rodriguez deivid-rodriguez marked this pull request as ready for review May 19, 2020 12:24
@deivid-rodriguez deivid-rodriguez changed the title Fix up the stubs to_spec whenever it is referenced Avoid stack overflow inside StubSpecification on some edge cases May 20, 2020
@deivid-rodriguez deivid-rodriguez merged commit 402f1be into master May 21, 2020
@deivid-rodriguez deivid-rodriguez deleted the fix_bundle_exec_bundle_install_git_extension branch May 21, 2020 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SystemStackError after upgrade to bundler 1.16
3 participants