OutOfMemoryError instead of Exceptions #216
Comments
|
Have any code to reproduce this? What do your JRUBY_OPTS and JAVA_OPTS look like? |
|
I believe this is related to a case where AR will attempt (during exception raising) to to_s a model or query object (or something) that has an extremely large amount of data backing it up. The result is that it consumes too much memory and never gets to the point of raising the actual error. |
|
I am not sure if we can solve this one. Perhaps giving more memory could help? I'm closing this for now; if there is a reproduction and any fabulous idea to go on with, we'll revisit. |
|
I'm seeing this same issue in my code right now. My code is handling images and other biggish files and it is quite common that when an exception is encountered the process consumes all the heap it has (3 Gigabytes currently) and then the Java heap space is thrown. I very much suspect the same as headius. What about instead of just to_s on the object we could take only the first 1000 characters or something. |
|
@mmustala: I'll drop this in the "ideas bucket." Consider me a crank until @headius weighs in: Far from an ideal solution, but for objects that can become this large... I'd perhaps suggest monkey patching For objects that can become really huge, overriding the behavior of |
|
I'm guessing this is actually to blame for the memory explosion on exception raise: https://bugs.ruby-lang.org/issues/9725 Not really sure how to avoid this; it's a weird misfeature of Ruby that it inspects the target of a NameError in this way. Maybe anyone affected by this should +1 the issue I filed. |
|
MRI has made no movement on the issue I filed, so I feel like we need to fix this unilaterally. We can mitigate this problem by catching OutOfMemoryError in the NameError logic and falling back on a simple class name. That could capture OOME unrelated to the string, but it's very unlikely (and if it's really OOM, another one will come along shortly). Beyond that, I think we should just modify NameError to not do this. @enebo? |
|
The problem with catching OOMError is it's inherently unreliable. If thread A is doing something insane (like building a ginormous string for an exception) and then thread B does a reasonable allocation at the wrong time then B will get an OOM and probably die a horrible death. I think your discussion on the MRI bug is spot on: you have to prevent the ginormous string allocation in the first place. Detecting the OOM after-the-fact will lead to even harder to diagnose problems, albeit more rarely. |
|
Hard to say if MRI would accept this change, since it would have dual behavior across Error objects. It helps that in the land of MRI, memory swap is freeeee! You should definitely solve this unilaterally. If it doesn't impact performance too hard, you could even juggle behavior based on the Obj's size... Or swap in a helpful String to let them know why there's no inspect output. |
|
There's no bug here for us to fix...it's an unfortunate side effect of standard Ruby behavior. JRuby 9.1 will not limit memory to 500MB, so perhaps it will rarely run into a NameError that's too big. If you would like to see this changed, add your vote to the CRuby bug: https://bugs.ruby-lang.org/issues/9725 |
f5d943e A few cosmetic fixes in Thread#backtrace spec 38cc7ef Basic tests of Thread#backtrace c062803 Test where class variables are defined and updated 9f94d6c Add a spec for adding two invalid strings together that results in a valid one 86ddccf Fixed Symbol.all_symbols spec. a98b62d Wait for a thread status change rather than a random amount of time in rb_thread_blocking_region 3dba3bf Merge pull request #219 from donv/basic_coverage_specs 2e7722e adapt realpath specs to run on Windows 6f1fbd4 syslog is POSIX-only d9b2e19 adapt realdirpath specs to run on Windows e80ca16 Process.daemon is not implemented on Windows 801850b Guard unicode path spec not working on Windows b6c738b Dir.home with an argument is not working on Windows aa55259 Update for changes in mspec's home_directory helper 25e443c Removing problematic path in dir specs for Windows 8f3e280 Show remaining files if delete_mock_dirs fails 83a11ee Merge pull request #216 from ruby/vais/windows 53b800f Skip all specs that rely on fork on Windows 76811e1 Wrap a few remaining fork-using specs in with_feature :fork block 937be98 Use explicit receiver for the helper method afbcef4 Change namespace odule to CoverageSpecs to avoid clashing with stdlib 0ecdf4d Guarded specs for peek_result for Ruby 2.3 Used raise_error helper instead of customer begin...rescue da7b44b Added specs for peek_result Pulled helper method to separate SpecHelper module 607e7ea Filter old results from result hash 3f65bbd Added specs for multiple runs c3e050f Added a basic spec for counts 490cc2d Merge pull request #217 from donv/try_add_coverage_spec dbaf24f Started adding spec for coverage 0f2743d Merge pull request #218 from donv/test_mri_2.3 86f19a1 Start testing MRI 2.3.x 07ea93b Fix indent 7e241f7 Fix typo 1a2d168 Reasonable precision in Bignum#* spec 342f9ae Merge pull request #215 from ruby/vais/windows 854cd89 Run AppVeyor only on master and try* branches fe5acba Passing all WIN32OLE specs e36ff82 Merge pull request #214 from ruby/vais/windows 4575e16 Skip the entire ARGF.read_nonblock spec on Windows f580a29 Merge pull request #212 from wied03/master 323d258 Cover another alias/super case 504eea0 Revert "Clean up the entire spec temp dir when finished with mock dirs." 8d75627 AppVeyor: use an inclusion filter 182a683 AppVeyor: fix last commit 4f24a03 AppVeyor: exclude C-API specs for now 40411ed AppVeyor: use the file formatter ea7d4ba Remove branch filter in appveyor.yml bfa4a13 Make sure the Signalizer is not run on Windows since it hangs 3bbc6ae Try to run the LeakChecker under AppVeyor a9c496a Add setup for AppVeyor 06e8a89 Merge pull request #210 from odaira/myContribution 9c17cb6 Merge pull request #211 from ruby/vais/windows 8b5c3eb Pass ENV specs on Windows 2307c18 Remove extra File.join in DirSpecs.nonexistent eaaf635 Module#{include,prepend} are public since 2.1 b144eac Remove 'ruby_version_is ""..."2.1" do' guards since 2.1 is now the lowest version 32c5020 Remove 'ruby_version_is "2.1" do' guards since this is now the lowest version c0b7368 Modify a test that succeeds or fails depending on today's date in a year. Time#gmtoff values match only when both or neither of the Time objects are in summer time 2458625 Drop support for 2.0.0 as it is no longer supported 6d4e6f4 Merge pull request #208 from ruby/vais/argf 52eff83 Pass ARGF.binmode on Windows e8c46eb Fix leak in Kernel#open 5080525 Choose a more appropriate error class in C-API thread specs c55e620 Fix 2 thread leaks in C-API specs 0c65308 Terminate the WEBrick timeout thread when supported 6d0be63 Fix thread leak in Thread#name spec ddf7d41 Show leaked subprocess pid c7a99a6 Revert "Remove now useless Process.waitall" 9676480 Fix subprocess leaks in ObjectSpace.define_finalizer specs 3229895 Do not leak a Dir in Marshal#load specs 4ff6dd5 Fix fd leak in Dir#fileno setup 32c48c7 Remove now useless Process.waitall 4d73980 Wait for subprocess to die in Process specs b513748 Close the pipe reading-side in the parent as well to avoid leaking a fd a277946 Unshare Process.times since there is only one usage c67efe7 Merge pull request #205 from unak/patch-4 ff2c878 get rid of blocking on Windows 5a1ea16 Merge pull request #204 from unak/patch-3 7a2f6b0 Should not assume to exist outer commands. 3b52dae Merge pull request #203 from unak/patch-2 87898e6 Wrong assumption adbe6d5 Make STDIN non-EOF add86e1 Revert "Remove problematic specs" 8b5cb71 Remove problematic specs 265cc4c fixup! remove unguaranteed behavior d8aadfb remove unguaranteed behavior 56d2e70 remove rb_class_inherited spec 4056f12 Remove unreliable spec 01bad6a Do not spec Unicode changing behavior 2258d1d Add missing 2.1 version guards. 3a80eb8 Add missing 2.3 version guards 8825b5f Add a spec for autoloading a constant that was already loaded by another thread 9be9a58 ObjectSpace._id2ref raises for invalid ID values 9d7f072 Move String#@{+,-} specs to the right files a579440 Added String#-@ and String#+@ 974a254 Remove redundant specs 6a44638 Added NameError#receiver spec e976989 Add specs for rb_time_timespec_new 1b4d6a2 Add specs for rb_define_class_id_under d61a24c Add a spec for rb_define_class_under when given a mismatched superclass 4e64448 add example which specs behavior of Proc#for_define_method when procs @ruby_method is set cba082b Added local variable methods to Binding 3c5e63d Use the argf helper in ARGF.read_nonblock 0938149 Added specs for ARGF.read_nonblock b960ebe Spec Enumerable#sort_by when #map does not return an array 51b21d0 Use describe, rephrase spec description 523f780 Fix count when enumerable yields multiple values 62c1c8c Reword spec description 309fda6 Add a spec covering Array#product returning self when given a block d9bbeb1 Add another failing specs for zsuper with keyword arguments 89e8773 Fix Kernel#rand when given a Float exclusive range. 08dd725 Added failing specs for zsuper with rest kwargs a828e0d Add failing specs for slice_* when iterator method is changed 7bb5997 Add specs for IO#{read,write}_nonblock accepting exception option 51fe64d Add specs for ARGF.readpartial f5b730c Add a spec covering clearing output buffer of ARGF.read 214cf9e add failing spec for String.new with optional encoding argument 48fd956 Move Hash comparison specs to the right files 7ed5840 Avoid the new_hash helper 9c43b34 Add specs for Hash comparison operators 4f7eaa5 Fixed some 64bit assumptions in specs. 1e84f38 Fix a typo in Enumerator::Lazy specs 48060b6 Add specs for File.mkfifo 1e971da Add specs for #dig with no arguments 59b2043 Add specs for Enumerator::Lazy#grep_v 38801cd Add specs for Hash#to_proc 93c40ee Add a spec for setting thread's group in rb_thread_create 668056f Add a spec covering rb_rescue returning the break value when the passed function yields to a block with a break 34bf4b5 Added 2.3 specs for #positive? and #negative? 3988aac multiple calls of close, close_read, and close_write should no longer raise IOError 065e422 Add specs for #dig implmenetation for Hash, Array and Struct classes 0be9395 puts: Make it more compatible to MRI e3c3858 spec 2.2 behavior where limit 0 raises ArgumentError d4f49e6 puts: Check if argument responds to :to_ary, instead of checking its type being Array cc7a472 Add spec for Enumerable#first with multiple yield parameters 7ca9596 Automatically run Travis on try* branches ba0bd69 Merge pull request #202 from wied03/master f6ed69a NoMethodError should not require a name (because underlying NameError class does not) 81977c9 sprintf: test infinite and nan float values 37d0d3c Merge pull request #201 from nobu/no-new_ostruct_member c800735 Do not test method for internal use git-subtree-dir: spec/ruby git-subtree-split: f5d943e19e980264de972b6d1b49c927e527053b
|
@headius Any suggestions for getting around this issue? Looks like we are facing this with JRuby 9.0.4.0 and Sidekiq. |
|
I didn't find anything else but overriding the |
|
Here is the stacktrace https://gist.github.com/prathamesh-sonpatki/0d6ce40f703a8ce930ff4b008af7002e |
|
I'm going to weigh in on this...I don't have any direct experience with this bug but I have been conversing with someone on Stack Overflow who does -- he gave up using JRuby, even though he needed to call Java libraries and it was the obvious choice, because of this problem (see comments in http://stackoverflow.com/questions/37316055/file-write-happening-asynchronously/37317141?noredirect=1#comment62158497_37317141). On the higher level, my opinion is that class authors should not presume to know how to_s and inspect are called, and support their use by the (Ruby or Java) VM and frameworks out of their field of influence by ensuring that the size of the strings they return is reasonable (maybe <= 1K?), and that the cost of calling them is low (e.g. not to read an entire DB table). I think we should be able to assume that we can call to_s or inspect to build a log message, for example, and it will not overpower the virtual machine. If a method that returns a large amount of data is needed, it can be implemented with a new name and called explicitly by those who have the specialized knowledge of it. So I wonder if we're looking in the wrong place for the solution. If ActiveRecord includes reads in an entire table and includes its data in a returned string, IMO this is the problem. If we assume that implementers of to_s and inspect should limit the size and cost of the returned strings, then it is reasonable to include them in an exception's text as is currently the case. However, given the severity of the situation, I think Charlie's suggestion is a quicker solution, and one that is more reliable, since we don't know what classes will be coded in the future. My vote would be to fix this ASAP, even if it means that JRuby deviates slightly from MRI. |
|
@keithrbennett are you sure that was the link you intended to include? I do not understand how it is related. As a general statement I find this to be bugs in various libraries who dump the planet. If you are going to dump a DB as output then perhaps you need to limit output? As a broader statement it also shows how Ruby might have been better off using IO-based API for this (e.g. output(print_io=$stdout)). I have a different proposal for to_s on NameError. I think arguably this is something where some people will want full output and others will not. We can make a new require: require 'abbreviated_to_s'
class NameError
include AbbreviatedToS
endBy including this to NameError we will check to see if it is included and then do a respond_to? :abbreviated_to_s and call that instead. This will put a possible burden on library author to fix the general issue by making their own abbreviated output. I think though if you plan on not including everything in output it is probably important that library auhors can make a best effort on what is displayed. A generic implementation can be made as well. It would need to be somewhat reflective and inspect sizes of various ivars and their values and elide values which are too big. So this could be something like: require 'abbreviated_to_s/reflective'
class ActiveRecord::Base
include AbbreviatedToS::Reflective
endThen if people run into this they can add a little magic in a Rails initializer and the problem is fixed. So on our side we would need NameError (or anything else which would need this) see if the module in question is in its hierarchy and call it instead of to_s if so. Someone would need to write these small shims and I think export them as a gem. This also feels like it could be baked into JRuby and MRI as well but perhaps just trying to solve this in one impl first might be a good path forward? Note: abbreviated_to_s and the names I picked are just stakeholder names. I do not really care what name is used so long as people like a name. |
|
@tenderlove @sgrif Might either of you know if this sort of problem has been fixed in ActiveRecord? To be more specific: a NameError raised against a model with a lot of relationships might drag along a LOT of output when the NameError is eventually printed out. See also https://bugs.ruby-lang.org/issues/9725 which has stalled. |
|
I don't believe so, no. I'm not sure if there's much that we can do on the AR side short of re-implementing |
|
I may be misunderstanding this issue, but... @sgrif Is it true that ActiveRecord's to_s is pulling in an entire table's data into the string? If this is so, then I would argue that the issue is not that NameError is calling to_s, but rather that the to_s is building and returning such a huge string to begin with. I think the higher level question that the community needs to answer is "given the havoc they can wreak, do we want to condone and support huge to_s strings?". In my opinion, the answer is no, because I think the value of enabling any code to be able to call to_s or inspect on unknown objects without fear of bringing down the system is greater than the value of the freedom to implement huge strings. @enebo The JRuby discussion was buried in the comments. Search the page for "JRuby" and you'll see it. Also, I'm sure your solution is technically sound, you know way more than I about these things, but I still think that's way more complexity than the freedom to return huge strings is worth. |
|
No we don't pull the entire table. On instances we actually shouldn't be grabbing anything from #inspect. On the class object we load the schema if it's not loaded. On a relation instance we will load the query that it represents. |
|
It seems possible the issues people have had here are not a problem in recent/current Rails...wish we could get some confirmation from someone who has had the issue in the past. |
|
I'd be happy to do more digging if you can point me at more details of what On Fri, May 20, 2016, 9:41 AM Charles Oliver Nutter <
|
|
I tried to create a simple app with the 9k jruby to demonstrate this error but couldn't get it reproduced. My original environment where the error happened is not available any more. I may give it another shot later if I find time for it. |
|
I no longer work on the project where I originally experienced the issue in question. I believe the problem was that activerecord added each field of the record in the to_s. I hope this helps.
|
|
I hope even without filing a new issue my comment will get attention. I am still facing the OP bug in JRuby 9.4.8.0 and it is essentially making development of our project a pain, since nameerrors might happen very often, and with this bug it is currently impossible to spot them in our code: The OOME do not show the user space location where the NameError is raised, in addition to the large time it takes to collect the inspect string. running this with: Looking purely on the jruby side, it is caused by: Ironic is that the inspect description is stripped out anyway if larger than 64 characters: ...so the larger the inspect string, the more time is wasted creating it for nothing ;-) @headius there is a comment in the standard repo https://bugs.ruby-lang.org/issues/18285#note-44 which I do not understand fully. Accordingly, https://bugs.ruby-lang.org/issues/9725 , which is essentially the ruby version of this issue should be fixed. But its still open, and the problem is still present in jruby ... How to understand that comment? Will we need to wait for a higher jruby version (using a higher ruby version) for this to get fixed? Would it be possible, like others suggested above, to fix this unilateral in jruby and just remove the inspect call? This would help alot for our case. best |
|
@mrckzgl Thank you for waking this issue up! You found all the right links and this is indeed fixed in standard Ruby version 3.3 and higher, but the current release line of JRuby, 9.4, is 3.1 compatible. I am willing to consider changing this behavior in a minor release of 9.4, but it will definitely be fixed for JRuby 10 due out this January. @enebo Would it be better to open a new issue or reopen this one? What do you think about making this change in 9.4.x, knowing that it's already standard behavior in CRuby? |
|
@headius let's open a new issue. Especially if we plan on this for 9.4. it
will be more clearly documented.
…On Wed, Oct 23, 2024, 9:52 AM Charles Oliver Nutter < ***@***.***> wrote:
@mrckzgl <https://github.com/mrckzgl> Thank you for waking this issue up!
You found all the right links and this is indeed fixed in standard Ruby
version 3.3 and higher, but the current release line of JRuby, 9.4, is 3.1
compatible. I am willing to consider changing this behavior in a minor
release of 9.4, but it will definitely be fixed for JRuby 10 due out this
January.
@enebo <https://github.com/enebo> Would it be better to open a new issue
or reopen this one? What do you think about making this change in 9.4.x,
knowing that it's already standard behavior in CRuby?
—
Reply to this email directly, view it on GitHub
<#216 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAE224AJICBHWRJQ5T5WADZ46ZZRAVCNFSM6AAAAABQO6DHFOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMZSGQ4TMMJQGM>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@mrckzgl could you take your comment above and turn it into a new issue? I think you've described the problem succinctly and provided a good example reproduction. The fix will be pretty easy. Hopefully the impact of doing it in 9.4 is extremely minimal. |
Hello, headius suggested i'd file a bug on this.
Sometimes when my code throws an exception instead it sits for over a minute and then throws an OutOfMemoryError.
This example trace is a ActionView::Template::Error but i guess ActionView catches the error and throws its own.
The text was updated successfully, but these errors were encountered: