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

Comparable#== raises SystemStackError when calling classes don't impleme... #2229



Copy link

@mjgpy3 mjgpy3 commented Nov 24, 2014

...nt #<=>

Yikes, this is embarrassing... Again trying for #1111

Copy link

mjgpy3 commented Nov 24, 2014

Not sure what I did wrong here, any help would be appreciated.

Copy link

headius commented Nov 24, 2014

The patch seems reasonable, but I wonder if we could more actively guard against recursion here? The main problem is that StackOverflowError can often happen in the middle of sensitive code, like building up some internal parts of a data structure. That makes those errors (and the SystemStackError we re-raise) generally fatal, since you can't rely on the state of the system.

Take another look at the MRI code in this patch, from the MRI bug I referenced in #1111:

Roughly equivalent code for us would be Ruby.recursiveListOperation and execRecursive, which you can see used for RubyArray.join19 logic. Have a look at that and try to do the same sort of recursion guard in RubyComparable.

Thanks for jumping in!

@enebo enebo added this to the Invalid or Duplicate milestone Dec 23, 2014
Copy link

enebo commented Dec 23, 2014

Closing since new version of PR exists

@enebo enebo closed this Dec 23, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
None yet

Successfully merging this pull request may close these issues.

None yet

3 participants