Skip to content

Fix exponential blowup in Pin::Method#combine_same_type_arity_signatures - #1236

Closed
apiology wants to merge 11 commits into
castwide:masterfrom
apiology:fix-combine-same-type-arity-exponential-blowup
Closed

Fix exponential blowup in Pin::Method#combine_same_type_arity_signatures#1236
apiology wants to merge 11 commits into
castwide:masterfrom
apiology:fix-combine-same-type-arity-exponential-blowup

Conversation

@apiology

@apiology apiology commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Fixes #1235.

Problem

Pin::Method#combine_same_type_arity_signatures (lib/solargraph/pin/method.rb) merges same-type-arity overload signatures via reduce + inner flat_map. The flat_map block returns the entire accumulator array from within a block scoped to a single element, so when N existing signatures fail to merge with a new one, the result balloons to N*(N+1) elements instead of N+1. That larger array becomes the accumulator for the next reduce step, so array size compounds multiplicatively with each additional signature — not the O(n^2) the existing guard comment assumes. The guard (return ... if same_type_arity_signatures.length > 10) only checks the initial array length and doesn't catch growth that happens incrementally inside the reduce.

In practice this causes solargraph typecheck --level strong to hang indefinitely with unbounded RSS growth when combining large families of same-type-arity overloads from RBS collection data (e.g. Integer#+, which RBS ships with many dispatch-by-type overloads). See #1235 for full repro steps, rbspy stack evidence (identical stuck stack across repeated samples), and RSS growth data on both macOS ARM64 and Linux x86_64.

Fix

Replace the per-element flat_map (re-emitting the whole accumulator per element) with a single Array#map pass: each element of old_signatures is independently combined-or-kept exactly once, and new_signature is appended only if nothing merged. This preserves the original intent (attempt to merge new_signature into each existing signature; keep unmerged ones as-is) and the intended O(n)-per-signature / O(n^2)-overall cost, without the accidental multiplicative blowup.

Testing

  • Added a regression test (spec/pin/method_spec.rb) using doubles that never successfully merge, verifying the combined result length tracks the input length (n) rather than exploding. This test would hang/OOM against the old code.
  • bundle exec rspec spec/pin/method_spec.rb: 48 examples, 0 failures (Ruby 3.4, Linux x86_64/Docker).
  • Ran the full CI recipe from .github/workflows/typecheck.yml (solargraph_typed job) against this branch on Linux x86_64: SOLARGRAPH_ASSERTS=on bundle exec solargraph typecheck --level strong now completes in ~6.5 minutes (previously hung indefinitely, RSS unbounded). Exit code was 1 with 49 problems found in 14 of 241 files — I believe this is unrelated gem/RBS-version drift from bundle update --pre rbs picking up whatever is latest as of today rather than what was current when v0.59's CI last passed, not a regression introduced by this change (the fix only removes duplicate/exploded array entries; it doesn't change which signatures successfully merge). Flagging this explicitly rather than asserting a clean typecheck, since I haven't diffed against a same-day baseline without this fix (that baseline hangs, by definition, so a like-for-like comparison isn't available).

apiology and others added 11 commits January 13, 2026 04:25
* Fix merge

* Adjust annotations

* Fix typecheck errors

* Fix merge

* Fix merge

* Fix some @sg-ignores

* Fix merge

* Merge branch 'intersection_types' into flow_sensitive_typing_2_0

* Fix RuboCop issue

* Fix type issues

* Fix type issues

* Fix rspec.yml

* Fix rspec.yml

* Add @sg-ignores

* Remove @sg-ignores

* Merge branch 'or_support_in_flow_sensitive_typing' into union_type_enforcement

* Fix spec

* Fix annotations

* Fix RuboCop issues

* Bump RBS versions in rspec test

* Fix version

* Fix version matrix

* Fix version matrix

* Fix version matrix

* Fix version matrix

* Fix version matrix

* Exclude another

* Exclude another

* Add version, fix doc

* init -> config

* Fix rbs-version for Ruby 4.0 in CI workflow

* Clean up ruby-version entries in rspec.yml

Removed deprecated ruby-version entries for RBS.

* Fix RBS version for Ruby 4.0 in workflow

* Fix merge

* Improve signature combination

Use our generated RBS signature from parameters as a key to combine
method signatures from RBS/YARD pins.

This is closer to what RBS does than the current technique of using
the arity alone, and fixes a key degenerate case in Integer#+ revealed
by updated definitions used by recently released RBS gems

* Update annotations

* Drop annotation

* Fix RuboCop issue

* Fix merge

* Don't use solargraph-rspec branch

* Fix merge

* Debug

* Add another use of stdlib dependencies in RBS

* Mock additional call

* Fix annotations

* Update types in rspec undercover

* Debug

* Debug

* Drop incorrect rbs collection use in spec

* Update rubocop todo

* Revert change

* Fix RuboCop issue

* Fix annotations

* Fix annotations

* RuboCop fix

* Use "type arity" to guide signature combination

* Update rubocop todo

* Include return type arity in comparison

* Add dodgy return type

* Fix RuboCop issue

* Add Ruby 4.0 jobs

* Exclude another combo

* Exclude another combo

* Update rules to use report?

* Fix merge

* Fix merge

* Drop dead code

* Bump version to 0.59.0.dev.1

* Rename rule

* Update RuboCop todo file

* Update RuboCop todo file

* Ratchet rubocop TODO file

* Move to skip:

* Mark spec as pending

* Revert spec change

* Drop old workaround

* Fix merge

* Fix typechecking issues

* Revert doc

* Fix spelling

* Fix merge issue

* Exclude the current gemspec from pins brought in from gem

* Check pathname instead

* Add sg-ignore

* Avoid rbs pollution

We were using the sig/shims directory for some internally helpful
shims; unfortunately that exported them during gem installs, causing
castwide#1144

* Test with RBS 4.0.0.dev.5

* Open up in gemspec

* Fix missing spot

* Typecheck using RBS prereleases

* Move point of ignoring cached gems for gem projects

* Fix issues resolving cgi escape functions

* Be more careful marking things as stdlib

* Reclassify rbs gem

* Fix merge

* Add sg-ignore

* Fix merge

* Remove outdated workaround

* Fix @sg-ignore name

* Restore workaround

* Restore workaround

* Merge branch 'flow_sensitive_typing_2_0' into 2025-01-06

* Fix method signature

* Fix annotations

* Add regression test and fix for issue found during future merge

* Add regression test and fix for issue found during future merge

* Fix merge

* Fix merge

* Fix merge

* Fix merge

* Use correct field for self type resolution

Add a regression test and fix for self type resolution issue found on
a future branch

* Fix 'solargraph pin --references ClassName' private method call

* Add error handling

* Fix another location with another test case

* Drop now-unneeded @sg-ignore

* Don't log caching for each dependent library

This causes duplicate logging on standard libraries, many of which are
esoteric (e.g., "cgi-escaping").  The current method as of the
2025-01-06 branch would result in each stdlib library being cached
individually.

* Drop logging entirely

* Fix some types based on future branch feedback

* Provide Gem::Specification to outside interface

* Provide Gem::Specification to outside interface

* Use #to_spec

* Provide Gem::Specification to outside interface

* Fix typechecking error

* Use consistent bundler versions

* Fix type issue

* Fix annotations based on future branch feedback

* Add some @todos

* Fix annotations

* Fix annotations

* Fix annotation

* Add diff::lcs shim

* Improve spec expectations

* Add @sg-ignore

* Fix rspec checks to run on all types of PRs

* Fix merge

* Fix merge

* Fix merge
* Improve signature combination

Use our generated RBS signature from parameters as a key to combine
method signatures from RBS/YARD pins.

This is closer to what RBS does than the current technique of using
the arity alone, and fixes a key degenerate case in Integer#+ revealed
by updated definitions used by recently released RBS gems

* Update annotations

* Drop annotation

* Fix RuboCop issue

* Fix merge

* Don't use solargraph-rspec branch

* Fix merge

* Debug

* Add another use of stdlib dependencies in RBS

* Mock additional call

* Fix annotations

* Update types in rspec undercover

* Debug

* Debug

* Drop incorrect rbs collection use in spec

* Update rubocop todo

* Revert change

* Fix RuboCop issue

* Fix annotations

* Fix annotations

* RuboCop fix

* Use "type arity" to guide signature combination

* Update rubocop todo

* Include return type arity in comparison

* Add dodgy return type

* Fix RuboCop issue

* Add Ruby 4.0 jobs

* Exclude another combo

* Exclude another combo

* Update rules to use report?

* Fix merge

* Fix merge

* Drop dead code

* Bump version to 0.59.0.dev.1

* Rename rule

* Update RuboCop todo file

* Update RuboCop todo file

* Ratchet rubocop TODO file

* Move to skip:

* Mark spec as pending

* Revert spec change

* Drop old workaround

* Fix merge

* Fix typechecking issues

* Revert doc

* Fix spelling

* Fix merge issue

* Exclude the current gemspec from pins brought in from gem

* Check pathname instead

* Add sg-ignore

* Avoid rbs pollution

We were using the sig/shims directory for some internally helpful
shims; unfortunately that exported them during gem installs, causing
castwide#1144

* Test with RBS 4.0.0.dev.5

* Open up in gemspec

* Fix missing spot

* Typecheck using RBS prereleases

* Move point of ignoring cached gems for gem projects

* Fix issues resolving cgi escape functions

* Be more careful marking things as stdlib

* Reclassify rbs gem

* Fix merge

* Add sg-ignore

* Fix merge

* Remove outdated workaround

* Fix @sg-ignore name

* Restore workaround

* Restore workaround

* Merge branch 'flow_sensitive_typing_2_0' into 2025-01-06

* Fix method signature

* Fix annotations

* Add regression test and fix for issue found during future merge

* Add regression test and fix for issue found during future merge

* Fix merge

* Fix merge

* Fix merge

* Fix merge

* Use correct field for self type resolution

Add a regression test and fix for self type resolution issue found on
a future branch

* Fix 'solargraph pin --references ClassName' private method call

* Add error handling

* Fix another location with another test case

* Drop now-unneeded @sg-ignore

* Don't log caching for each dependent library

This causes duplicate logging on standard libraries, many of which are
esoteric (e.g., "cgi-escaping").  The current method as of the
2025-01-06 branch would result in each stdlib library being cached
individually.

* Drop logging entirely

* Fix some types based on future branch feedback

* Provide Gem::Specification to outside interface

* Provide Gem::Specification to outside interface

* Use #to_spec

* Provide Gem::Specification to outside interface

* Fix typechecking error

* Use consistent bundler versions

* Fix type issue

* Fix annotations based on future branch feedback

* Add some @todos

* Fix annotations

* Fix annotations

* Fix annotation

* Add diff::lcs shim

* Improve spec expectations

* Add @sg-ignore

* Fix rspec checks to run on all types of PRs

* Fix merge

* Fix merge

* Fix merge

* Avoid rbs pollution (castwide#1146)

We were using the sig/shims directory for some internally helpful
shims; unfortunately that exported them during gem installs, causing
castwide#1144

* Fix 'solargraph pin --references ClassName' private method call (castwide#1150)

* Manual rubocop fixes

Some hand-changed fixes for RuboCop issues

* RuboCop manual fixes

* RuboCop manual fixes

* Improve memory efficiency of Position class (castwide#1054)

* Use each_line instead of text.lines

Avoid allocating additional strings, instead use sliced substrings

* gitignore vendor/cache

* Remove redundant end_with?

Co-authored-by: Jean Boussier <jean.boussier@gmail.com>

* Remove benchamrks

castwide#1054 (comment)

* String#index(offset:) FTW 🚀

castwide#1054 (comment)

Co-authored-by: Jean Boussier <jean.boussier@gmail.com>

* fix rubocop

---------

Co-authored-by: Jean Boussier <jean.boussier@gmail.com>

* RuboCop manual fixes

* RuboCop manual fixes

* RuboCop manual fixes

* RuboCop manual fixes

* RuboCop manual fixes

* RuboCop manual fixes

* RuboCop manual fixes

* RuboCop manual fixes

* Raise InvalidOffsetError for offsets > text (castwide#1155)

* Raise InvalidOffsetError for offsets > text

* Linting

* Fix fencepost error

* Additional fencepost test

* Document exception

* Refactor RbsMap::Conversions

Let's work more directly with type objects and reduce duplication

* Release 0.58.2

* Complete other_type_to_type transition

* Refactor

* Refactor

* Refactor

* Refactor

* Refactor

* Refactor

* Refactor

* Refactor

* Add @sg-ignores

* Fix tuple issue

* Refactor

* Tuple -> Array()

* Add @sg-ignore

* Use rooted names, clarify intent

* Refactor

* Remove TODOs, add asserts

* Fix solargraph-rspec spec failure

* Fix merge

* Adjust rubocop todo

* Debug logging fixes

* Reproduce build problem with RBS pre-release

* Stop hard-coding bundler version

* Use bundler preferred by setup-ruby step

* Fix merge

* rubocop -a

* manual typechecking fixes

* Fix indentation

* Sync @sg-ignores with CI

* rubocop -A

* Fix typechecking issues

---------

Co-authored-by: Lekë Mula <l.mula@finlink.de>
Co-authored-by: Jean Boussier <jean.boussier@gmail.com>
Co-authored-by: Fred Snyder <fsnyder@castwide.com>
…de#1172)

* Allow vernier gem to be used optionally, but don't require it

* Add gem for typechecking

* Add gem for typechecking

* Apply suggestion from @apiology
* Position linting

* Position spec linting

* Redundant tests

* Server notifications use $stderr.puts instead of warn

* Add sg-ignore

* More sg-ignore tags

* Move sg-ignore tag
* Revert YARD and RBS pin caching

* Update specs

* Fix argument

* Shell specs for unbundled environments

* Linting

* Linting

* Skip reason

* Split long string

* Ignore missing alias in RuboCop RBS

* Type alias reference pins (castwide#1181)

* Type alias reference pins

* Type tags

* Fix method name

* Typify with type alias pins

* Rooted aliases

* TypeAlias source

* TypeAlias type_location

* Fix TypeAlias return type

* Update sg-ignore tags

* Fix Pin::Base#typify format

* Skip Ruby < 3.4 on flaky test

* Switch to RBS < 4

* Stub failing test

* Test requires RBS >= 3.10.0

* Exclude Ruby 3.0 and RBS 3.10.0

* Skip the test

* Stale type aliasing

* Linting

* Fix dependency resolution

* Errant autoload

* Skipped test

* Strong typechecking errors

* Linting

* Linting and typechecking

* Typecheck errors

* Linting

* Typecheck

* Use rbs 4.0.1

* Allow nested methods like `Hash::_Key`

* Unalias unique types

* Fix ApiMap mock

* Typechecking

* Remove sg-ignore for Vernier
…ide#1183)

* Unbundled environment tests emit error messages

* Force require pathname
combine_same_type_arity_signatures merges same-type-arity overload
signatures via Array#reduce, with an inner Array#flat_map over the
accumulator (old_signatures). On each element of old_signatures, the
block's non-merging branch returned the *entire* old_signatures array
(plus the new signature) instead of just that element's contribution.
Since flat_map concatenates every branch's return value, when N
existing signatures fail to merge with a new one, the result becomes
N*(N+1) elements instead of N+1 -- and that larger array becomes the
accumulator for the next reduce step, so the size compounds
multiplicatively with each additional signature.

The existing "bail out if n is not small" guard only checks the
length of the *initial* input array, so it doesn't catch this blowup,
which happens incrementally inside the reduce.

In practice this caused `solargraph typecheck --level strong` to hang
indefinitely with unbounded memory growth (confirmed via rbspy
live Ruby-level stack sampling: the process was stuck in an
identical stack at pin/method.rb:532-533 across repeated samples,
with RSS climbing continuously and no forward progress) when
combining large families of same-type-arity overloads pulled in via
RBS collections (e.g. Integer#+).

Fix: replace the per-element flat_map (which re-emits the whole
accumulator from within a block scoped to a single element) with a
single Array#map pass that independently combines or keeps each
element once, appending the new signature only if nothing merged.
This keeps the same O(n) per-signature, O(n^2) overall behavior the
original guard comment describes, without the accidental
multiplicative growth.

Adds a regression test using doubles that never successfully merge,
verifying the combined result size tracks the input size rather than
exploding.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@apiology

apiology commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

This PR targeted the v0.59 tag, which has diverged from master significantly (different commit lineages, ~46 commits on master not in v0.59 and 10 commits on v0.59 not in master since their common ancestor) — hence the huge conflict set here.

More importantly, current master already stubbed out combine_same_type_arity_signatures entirely in #1186 as a stopgap for this exact hang ("debug an infinite loop bug in Ruby 3.x"), so the code this PR patches is currently dead on master. Superseding this with a fresh PR based on current master that removes the stub and applies the real fix: see the new PR (linked from #1235).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

solargraph typecheck --level strong hangs indefinitely (exponential blowup in Pin::Method#combine_same_type_arity_signatures)

2 participants