-
Notifications
You must be signed in to change notification settings - Fork 131
Fix lastNoSuccessVar memory leak #69
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
Conversation
Fixes https://issues.scala-lang.org/browse/SI-9010, see https://issues.scala-lang.org/browse/SI-4929 for discussion. The leak happened when parsers are used outside of phrase, which seems to be not so common until one realizes that that's exactly how lexers are used in Scanner, so every lexer leaked. This should fix it.
You could smoke test it using ParserLoop.scala from https://issues.scala-lang.org/browse/SI-9010 (or derive a unit test from it). I think idea was to make a Phrase parser with a input that fails(Failure),so that the NoLastSuccess logic gets activated. |
Well, ParserLoop has been running for several minutes and all is well. Brilliant. \o/ But I'm not sure how to derive a unit test. I don't suppose I can limit the heap size of a thread in a shared memory virtual machine. And lastNoSuccessVar is private. |
And it's been running for almost an hour and still good. :-) |
This is extremely ugly (not only does it use reflection, is uses Java reflection with hard-coded mangled symbol name because of a bug in Scala reflection), but I don't know of a better solution. :-(
LGTM The test isn't very nice, but until SI-9306 is fixed it will have to do. Hopefully that bit of name mangling won't change in 2.12.x, or we'll have to get fancy to support both versions... |
Fix lastNoSuccessVar memory leak
🎉 great to see this long story winding to a close. (oops, I just jinxed it...) |
Fixes https://issues.scala-lang.org/browse/SI-9010, see
https://issues.scala-lang.org/browse/SI-4929 for discussion.
The leak happened when parsers are used outside of phrase, which seems to
be not so common until one realizes that that's exactly how lexers are used
in Scanner, so every lexer leaked. This should fix it.
(I haven't tested this yet, just coded the idea I got in the bath. And
now the obvious question, how do I test this without visualvm, i.e. in a
unit test? I don't want to make lastNoSuccessVar non-private just for
the sake of testing. Can I look at it using reflection or something?)