Comments and changes to this ticket

• 

  Michael Koziarski November 7th, 2008 @ 07:39 AM

  • Assigned user set to “Michael Koziarski”

  Git blame + git show points those lines as fixing a memory leak in dev mode:

  http://github.com/rails/rails/co...

  I believe we removed that test case, but there's the reason.

  Can you confirm that there's no leak in dev mode after applying this patch?

• 

  Michael Koziarski November 7th, 2008 @ 07:50 AM

  for additional information, the leak previously was caused by the use of define_method() and reloading. The procs used by define_method were held on to indefinitely, despite them not being in use any more.

  If this has been fixed in a recent ruby version, then perhaps it's not a big deal. But it was a complete disaster previously.

• 

  josh November 10th, 2008 @ 06:42 PM

  • Milestone cleared.
  • State changed from “new” to “open”

  Can we make this a 2.2 priority?

• 

  Michael Koziarski November 10th, 2008 @ 07:56 PM

  • Milestone set to “2.x”

  Not with the current implementation.

  At present it does not affect anyone running with cache_classes = true, and it leaks (or leaked, no one has done more testing) when the patch is applied.

  This is an old issue, and shouldn't block a release, though it'd be nice to have it fixed obviously :)

• You flagged this item as spam.
  

  Steven Soroka November 10th, 2008 @ 08:22 PM

  I disagree with Michael Koziarski. This has caused a lot of headaches for a lot of people, and it would be nice if it could be fixed for the next release.

  See http://rails.lighthouseapp.com/p... for a long list of complaints. :)

• 

  Michael Koziarski November 10th, 2008 @ 08:26 PM

  I don't disagree that this is an annoying issue, but the fix in the other ticket to add a marshalling memory store does just as good a job as this patch, without leaking memory.

  If you want to help out here, you should test this current patch against the current major versions of ruby (1.8.6, 1.8.7 and whatever debian stable is on) and report back if the code leaks memory in development mode.

• You flagged this item as spam.
  

  Tom Lea November 12th, 2008 @ 07:24 PM

  So I did a little memory profiling, it was a little hacky, so my methods may be bad. If this is so, tell me how to do it better.

  I restarted the server each time, and switched branches when appropriate.

  ruby -v

  ruby 1.8.6 (2008-03-03 patchlevel 114) [i686-darwin9.4.0]

  (I ran the test against 1.8.7, freshly downloaded today, and the result were almost identical, 1.9{,.1} fail to start due to #827 on 1.9 and some other issue that ran over the end of my lunch hour on 1.9.1)

  Test Script

  
  #!/usr/bin/env ruby
  
  pid = `ps | grep script/serve[r] | awk '{ print $1 }'`.chomp
  puts "Server PID: #{pid}"
  
  url = ARGV.last
  puts "URL: #{url}"
  
  puts "Hit enter to go."
  $stdin.gets
  
  def get_memory_usage(pid)
    `ps -o rss,vsz -p#{pid} | tail -n1`.chomp
  end
  
  puts "Initial Memory Usage: #{get_memory_usage(pid)}"
  
  1000.times do
    system %Q{wget -O/dev/null -o/dev/null #{url}}
  end
  
  puts "Final Memory Usage: #{get_memory_usage(pid)}"
  
  

  Without Patch

  
                          RSS      VSZ
  Initial Memory Usage:  31568   107692
  Final Memory Usage:    43400   128408
  Delta:                 11832   20716
  
  Initial Memory Usage:  31568   107692
  Final Memory Usage:    43460   128628
  Delta:                 11892   20936
  
  Initial Memory Usage:  31568   107692
  Final Memory Usage:    43320   128248
  Delta:                 11752   20556
  
  Mean Delta:            11825   20736
  
  ## With Patch
  
  Initial Memory Usage:  31568   107692
  Final Memory Usage:    43584   128308
  Delta:                 12016   20616
  
  Initial Memory Usage:  31564   107692
  Final Memory Usage:    43656   128468
  Delta:                 12092   20776
  
  Initial Memory Usage:  31612   107736
  Final Memory Usage:    43644   128356
  Delta:                 12032   20620
  
  Mean Delta:            12046   20671
  

  Conclusion

  So that's a 221k difference over 1000 requests, or an additional 226bytes per request leakage in development, no change in production. This is likely to be more on more complex apps, although I have not had time to do any more in depth testing.

  IMHO fixing leakage in this specific way in development mode is kind of pointless, given that the exiting leakage by far out weighs it.

  Or taken another way, if the inverse of the patch were proposed, I would object to it, as it would cause the issues we are seeing now.

  Opinions please?

• 

  Michael Koziarski November 13th, 2008 @ 09:39 AM

  • Milestone cleared.
  • Assigned user changed from “Michael Koziarski” to “Jeremy Kemper”

  Moving to 2.2 so we can make a final call on this.

  Jeremy, You've done a lot of our memory profiling lately, what's your take?

• 

  Michael Koziarski November 13th, 2008 @ 09:41 AM

  What was the application you were testing, how many AR models were involved? It's AR which makes extensive use of define_method, especially in associations?

• You flagged this item as spam.
  

  Tom Lea November 13th, 2008 @ 09:55 AM

  Test app was [1] with a new action that just returned a blank page.

  So not a very good test at all.

  As mentioned, this was a bit of a botch job.

  I'll try and do some more testing on a more representative app soon. Any idea what this app should involve? Or do we have a good standard test case?

  [1] http://rails.lighthouseapp.com/a...

• 

  DHH November 13th, 2008 @ 10:08 AM

  • Milestone set to “2.x”

  I think it would be great to get this fixed. But we need to make sure we're not just adding back the leaking headache. I remember before we had this and it wasn't pretty. You'd have to restart your dev server pretty often on a complex app.

  Perhaps this is no longer valid, but we should absolutely confirm that before including it. So I say we don't aim for 2.2 final, but as soon as we have a confirmation, we put it in there.

• You flagged this item as spam.
  

  Gunnar Djurberg December 7th, 2008 @ 12:17 AM

  Guys, I get the exact same error as described at the top of ref2 (duplicate ticket) using ActiveRecord::Base#hash. (First I do Object.const_set(name, AR_subclass then, when running AR#hash a second time - after reload - it happens.)

  Can anyone help out a newcomer that don't understand the suggestions above: - Should I apply the patch and how? Am I supposed to edit the source to try replicate the diff? - Is there a workaround? If so, where can I find instructions? - If no, shouldn't this be high prio ticket, or is this weird/unusual use of RoR? - The difficulty of understanding what is going and the impression of side-effects is making a newcomer a bit worried: Array#uniq fails because AR#hash fails after reload because AR works differently when defining subclasses "dynamically" - none of which is easy to see or explained anywhere. etc etc

  I use Rails 2.2.2 on jruby 1.1.5 (ruby 1.8.6 patchlevel 114) (2008-11-03 rev 7996) on Java 1.6 and win XP

• You flagged this item as spam.
  

  C. Bedard January 6th, 2009 @ 07:13 PM

  Having encountered this problem myself (and being stuck on it repeateadly) I have investigated the error (and hopefully found a good fix). Here's what I know about it:

  1. It happens when ActiveRecord::Base#reset_subclasses is called by the dispatcher between requests (in dev mode only).
  2. ActiveRecord::Base#reset_subclasses wipes out the inheritable_attributes Hash (where #skip_time_zone_conversion_for_attributes is stored).
  3. It will not only happen on objects persisted through requests, as the "monkey test app" from #1290 shows, but also when trying to access generated association methods on AR, even for objects that live only on the current request.
  4. This bug was introduced by this commit where the #skip_time_zone_conversion_for_attributes declaration was changed from base.cattr_accessor to base.class_inheritable_accessor. But then again, that same commit also fixed something else.
  5. The patch initially submitted here that simply avoids clearing the instance_variables and instance_methods in reset_subclasses does introduce massive leaking, and the amounts leaked seem directly proportional to complexity of the app (i.e. number of models, associations and attributes on each of them). I have a pretty complex app which leaks nearly 1Mb on each request in dev mode when the patch is applied. So it's not viable (for me anyways).
  6. While trying out different ways to solve this, I have corrected the initial error (skip_time_zone_conversion_for_attributes being nil on 2nd request), but it uncovered another error (which just didn't happen because the first exception would be raised before getting to it). That error seems to be the one reported in #774 (Stack overflow in method_missing for the 'id' method).

  Now, for the solution, my patch (attached) does the following:

  1. It adds wrapper methods for #skip_time_zone_conversion_for_attributes methods, making sure it always reads/writes the value as an class_inheritable_attribute. This way, nil is never returned anymore.
  2. It ensures that the 'id' method is not wiped out when reset_subclasses is called. AR is kinda strange on that one, because it first defines it directly in the source, but redefines itself with #define_read_method when it is first called. And that is precisely what makes it fail after reloading (since reset_subclasses then wipes it out).

  I also added a test in reload_models_test.rb, which calls reset_subclasses to try and simulate reloading between requests in dev mode. What I cannot tell at this point is if it really triggers the reloading mechanism as it does on a live dispatcher request cycle. I also tested from script/server and the error was gone.

  Comments, opinions (and probably further testing) are welcome.

  Regards,

  • AR_dev_mode_reloading.diff 4.3 KB
• 

  Geoff Buesing January 7th, 2009 @ 05:59 AM

  I think it's fine to add getter and setter wrapper methods around skip_time_zone_conversion_for_attributes -- this would remove the need to use an explicit +self+ receiver whenever you need to declare these attributes in a model -- and it does address some of the issues found with calling methods on cached models: i.e., you can now call method_missing-defined methods without failure.

  But keep in mind, with or without this fix, there's still a problem with user-defined methods on a model being removed -- in the Monkey app, the Monkey model defines a #> method (it could be named #foo or whatever) and this method fails on the second page refresh, with or without a skip_time_zone_ fix.

• You flagged this item as spam.
  

  Emili Parreño January 7th, 2009 @ 08:20 AM

  I get the same error too, only in development mode.

  Environment: Mac OSX 10.5, Rails 2.2.2, Mongrel 1.1.5, ruby 1.8.6 (2008-03-03 patchlevel 114)

  In my case, the error appears when the app call a observer. I fix it changing "config.time_zone = 'Madrid'" for "config.active_record.default_timezone = 'Madrid'" in environment.rb

  In my case the solution proposed by Geoff in #1290 doesn't work.

• You flagged this item as spam.
  

  C. Bedard January 7th, 2009 @ 03:52 PM

  In my view, the Monkey app, while it does produce our error, uses the Rails framework in a specific way that is more related to persistence across requests than standard use of an AR instance.

  The intended and expected behavior of the framework is to allow to create, use and dispose of an AR object within a single request (and repeat that consistently on each request).

  I'm not saying that Rails shouldn't provide functionality to allow to do what the Monkey app does (which it already does if serialization was used), but that would represent an addition to the framework (as opposed to currently being a bug on expected behavior).

  It would be very nice to come up with a way that addresses both problems at once, but the way the reloading works does not allow that, because it creates new class instances on each request, instead of stripping a class instance from its methods and attributes and re-applying them to the same instance.

  In the case of the Monkey app, if MONKEY_ONE is to be initialized that early and be kept throughout the life of the application, auto-reloading can (and should) be avoided with a simple explicit require (in extensions.rb) :

  @@@ruby require File.join(RAILS_ROOT,'app/models/monkey')

  MONKEY_ONE = Monkey.first

  
  
  And now the Monkey is not so amnesic anymore ;-)
  
  `ActiveSupport::Dependencies` only autoloads missing classes (constants) and the dispatcher only unloads the autoloaded classes in `#cleanup_application`. This is why, for example, classes defined in plugins do not unload automatically.
  
  Of course, there may be other issues with reloading that we are not aware of, and I'll be glad to help if anything else comes up.
  

• You flagged this item as spam.
  

  C. Bedard January 7th, 2009 @ 03:56 PM

  Sorry about the poor formatting in my previous message...(A preview function would be nice in here :-S ).

• 

  Geoff Buesing January 7th, 2009 @ 06:01 PM

  @Emili -- config.active_record.default_timezone represents the zone that times are persisted in -- you can only set this to :utc or :local; 'Madrid' is not a valid setting. If you're using config.time_zone, you don't need to set active_record.default_timezone, because this will automatically set it to :utc for you.

  With config.time_zone = 'Madrid', and with no explicit active_record.default_timezone setting, are you still experiencing issues?

  @C. Bedard -- one thing that's not clear for me: in your previous comment, you refer to an issue "when trying to access generated association methods on AR, even for objects that live only on the current request" Do we have an example of this issue?

• You flagged this item as spam.
  

  C. Bedard January 8th, 2009 @ 05:36 AM

  • Tag changed from “activerecord, bug, reload” to “activerecord, bug, dependencies, reload”

  @Geoff:

  I have made an example (attached) that triggers the error without persisting AR objects between requests. I try and explain as cleary as possible what goes on (from what I understand) in the app README.

  Basically what happens is that a file that is explicitely required (with require 'file'), it will not be reloaded on each request, while the class it declares is being wiped out by ActiveRecord::Base#reset_subclasses. On 2nd request, the class remain "empty"...

  Obviously, the whole thing can be avoided by never using require in files that declare AR subclasses. But there can be contexts in which explicit requires are needed (I have an app for which it is the case).

  • lighthouse-ticket-1339.zip 92 KB
• 

  Geoff Buesing January 8th, 2009 @ 03:44 PM

  @C. Bedard -- thanks for the example, very helpful to have this. I think you're right about the explicit require mucking up the works with Dependencies and class reloading.

  I did some experiments with the app, here's what I found:

  1. with the require for another AR-model declared at the top of Monkey, you get an exception on the second request (as promised.) The stack trace stops at a NoMethodError for skip_time_zone_

  2. without the require, things work fine (again, as promised.)

  3. if I move the user_role require from Monkey to the bottom of environment.rb, I experience the same issue on the second request.

  4. with the require, but if I add this method to each of the models:

  
  def self.skip_time_zone_conversion_for_attributes
    []
  end
  

  things work fine. This is similar to what your patch does, i.e., work around the class inheritable accessor issue with skip_time_zone_.

  HOWEVER:

  1. with the require, and with skip_time_zone_ explicitly declared as a class method, if I declare a method #foo in Role, and then I change this line in the view from:

  
  # calling a Role attribute method defined by method_missing:
  u.user_roles.collect { |ur| ur.role.description }.join(", ")
  

  to

  
  # calling a method explicitly defined in Role class:
  u.user_roles.collect { |ur| ur.role.foo }.join(", ")
  

  ...I get a NoMethodError on the second request -- the stack trace tops out at method_missing.

  So, it seems like this reload issue is very similar to the issue with objects cached between requests -- there's the class inheritable accessor issue with skip_time_zone_, but even if you work around that, there are issues with calling methods that aren't regenerated by method_missing.

  Possible solutions for these problems:

  1. for the issue with objects cached between requests -- we can instruct users to always use Rails.cache with a marshaling memory store

  2. for the explicit require issue -- we can instruct users to never explicitly require an AR model when reloading is desired, as you suggest.

• 

  Geoff Buesing January 8th, 2009 @ 04:05 PM

  @C. Bedard -- in regards to your patch: by working around issues with #id and #skip_time_zone_, you may just be pushing the problem further downstream, instead of truly fixing things.

• You flagged this item as spam.
  

  C. Bedard January 8th, 2009 @ 04:55 PM

  @Geoff:

  Indeed my patch doesn't solve the problem. I didn't think at that time that it was related to dependencies.

  If skip_time_zone_for_attributes actually contained some fields, they would be cleared after 1st request and since the class would never reload on 2nd request, it would be silently reset to an empty array. Same goes for any other method (my patch actually dealt with the id method also, but that was a short-sighted fix since all methods are undef'ed).

  And I would add to your second suggestion:

  If an explicit require is needed, then those model files should be loaded on app initialization and users would need to understand they lose the reloading functionality for those models in dev mode.

  As for you suggestion to persist objects to the Rails.cache, I am not too familiar on how it works, but how would an object cope with being tied to a Class that has been stripped from its method declarations? Does the Class instance get cached too? And if so, isn't it the same as not reloading it?

• You flagged this item as spam.
  

  Emili Parreño January 10th, 2009 @ 06:49 AM

  @Geoff: thanks for the explanation. I experiencing issues everytime that I set config.time_zone = 'Madrid' (I tried to set the time_zone = 'Paris' and I experiencing the same problem). If I comment this line and start the app with active_record.default_timezone :local or :utc, all works ok.

• 

  Geoff Buesing January 11th, 2009 @ 08:53 PM

  @C. Bedard -- if the Rails.cache memory store marshaled objects before storing, and unmarshaled them on retrieval, then it would never return instances of stripped-down classes: unmarshaling would re-create instances with current class definitions.

  @Emili: to be clear, all works ok if you avoid calling methods that can't be generated via method_missing on cached instances.

• 

  Michael Koziarski January 12th, 2009 @ 06:30 AM

  Perhaps the best bet here is to provide a patch which makes just the one potentially leaking change. Ideally one that applies cleanly to 2.2

  We can then ask people to run their apps in development with it turned on and report the numbers back

  It'd be great to fix this finally, and to do so knowing we're not going to be wasting too much ram

• You flagged this item as spam.
  

  Tom Lea January 12th, 2009 @ 08:46 AM

  @Koz would that be a patch along the lines of the original one attached to the ticket? or something else? I'm happy to rebase it if that is the case. It is definitely leaky in dev, but just how leaky in a large app is still unknown.

• 

  Michael Koziarski January 12th, 2009 @ 09:10 AM

  Yep, if it still applies cleanly to master and/or 2-2-stable just let me know.

• You flagged this item as spam.
  

  Tom Lea January 12th, 2009 @ 10:28 AM

  The attached patch applies cleanly to both master and 2-2-stable, I've updated the commit note to be a bit more descriptive, but obviously feel free to edit.

  • rebased-20090112-prevented-AR.reset_subclasses-from-killing-existing.patch 1.3 KB

• You flagged this item as spam.
  

  C. Bedard January 12th, 2009 @ 05:27 PM

  @Geoff: You're saying "...then it would never return instances of stripped-down classes: unmarshaling would re-create instances with current class definitions."

  From what I understand, the "current class definitions" after reloading are stripped-down. So if an explicit require (as in my example app) was used (mocking up the Dependencies autoload), marshalling an instance would not change the fact that its Class has lost its methods since it is not reloaded.

  @Michael: I have tested the patch and as I mentioned earlier, it does leak in dev mode. Here are my test results (same app, same request 100 times in a loop):

  With patch: Initial memory usage: 40 092 Kb after 100 requests : 120 104 Kb

  Without patch: Initial memory usage: 40 168 Kb after 100 requests : 61 824 Kb

  The leak seems to be proportional to the number of methods and attributes of all AR subclasses (which makes sense since it is the instance_vars and instance_methods that do not get cleared anymore).

  Note that memory usage keeps growing in dev mode regardless of the patch (probably pointing to other leaks in dev mode (?).

• You flagged this item as spam.
  

  C. Bedard January 12th, 2009 @ 06:11 PM

  Also, come to think of it: Tom's patch, by not stripping down the subclasses, does not address the underlying issue (which is really that an AR subclass might not be reloaded on each request because Dependencies has skipped it if it was explicitly required somewhere). So basically, the error is not raised anymore because the app holds on to the initial class definition which is not being stripped-down anymore.

  As my patch did, it only hides the fact that the Class is not being reloaded.

  You can try this with the Monkey App with the patch applied: Launch the app, hit it with a first request, then remove the def >(object) method. If you hit the server again with more requests, no error is raised, and the > method is still defined (which it should not if the class has been reloaded). MONKEY_ONE.class == @monkey_two.class # => false

• You flagged this item as spam.
  

  Tom Lea January 12th, 2009 @ 06:43 PM

  @C. Bedard: Just to be clear, the patch does not make magic happen, all instances remain instances of the class they always were instances off. When the contsants that reference the classes (the constant Monkey in this case) are undefined by Dependencies, the instances still refer to the classes they were instantiated from.

  This is how it works with standard classes (Object.remove_const is effectively the reload):

  
  class Foo
    def bar
      10
    end
  end
  
  baz = Foo.new
  baz.bar #=> 10
  
  Object.send :remove_const, :Foo
  
  class Foo
    def bar
      20
    end
  end
  
  baz.bar #=> 10
  
  bam = Foo.new
  bam.bar #=> 20
  

  baz is and always will be an instance of the first Foo (from before the reload), and this is always the case. It cannot be changed and will refuse to mutate.

  The same is true of active record classes. The only difference here is that special code (that code removed by the patch) strips out the original instance methods, and leaves the instances disfigured and unpleasant to the touch.

  The class has been redefined, and new instances will behave as expected, but existing instances will be different.

  Another example to further the point that Monkey and Monkey after the reload are different entities:

  
  class Foo
  end
  
  foo_one = Foo.object_id
  
  Object.send :remove_const, :Foo
  
  class Foo
  end
  
  foo_two = Foo.object_id
  
  p :foo_one => foo_one, :foo_two => foo_two
  
  # >> {:foo_one=>1701840, :foo_two=>1701810}
  

  So in conclusion, classes are not reloaded, they are recreated. Making them reload is for another time and another place... and is not easy looking.

  p.s. If I lost my point somewhere in there, it's because it's 6:42pm here, and I've had a long day of I18n cards ;).

• You flagged this item as spam.
  

  Christian Gregertsen January 25th, 2009 @ 07:08 AM

  This caused me a major headache. I am modifying Community Engine and was getting first the time zone error, then I set environment to use local time and I got random NoMethod on second page load on methods defined in my models. I tried the patch and it fixes my issue.

• 

  Michael Koziarski January 26th, 2009 @ 03:19 AM

  I see similar leak statistics to What C. Bedard mentioned. 60M after 100 requests.

  This isn't an acceptable cost to pay for this problem sorry, it's around 600k per request for an app with only around 15 models and associated associations. So we need another fix which doesn't leak that memory.

  Another option would be to make the 'vestigial classes' have a method_missing which showed a very detailed error message about what happened, and what they can do about it.

• You flagged this item as spam.
  

  Frank Wöckener February 9th, 2009 @ 02:38 PM

  Even after applying the patch I get an error at the 1st reload of a page, when I include a related model in the caching:

  class Project < ActiveRecord::Base ... belongs_to :person ... def self.all_cached Rails.cache.fetch('Project.all_sort_name') { Project.sname_sort } end

  def self.sname_sort Project.all(:include => 'person', :order => 'people.sname ASC') end

  This gives me a

  Argument error "undefined class/module Person"

  after reloading (2nd display) in development mode. This doesn't happen in production mode, or after terminating/restarting memcached which is used as caching server.

• You flagged this item as spam.
  

  Paweł Kondzior March 6th, 2009 @ 07:05 PM

  What's the status of this bug ? I have this problem in JRuby environment.

  Can we just add :default param for default value of class_inheritable_accessor ?

  example:

  
  base.class_inheritable_accessor :skip_time_zone_conversion_for_attributes, :instance_writer => false, :default => []
  

• You flagged this item as spam.
  

  Tom Lea March 6th, 2009 @ 10:21 PM

  @pawel: As far as I am aware, this is still unresolved.

  The 'real world' memory leaks reported are unacceptable. Much bigger than I expected, so this solution is officially pants.

  Sorry I've gone a little 'dark' on this one, as I am basically out of ideas, and am no longer feeling the hurt at work. The 'vestigial classes' solution is not one I am a particularly large fan of, but it may well be the only suitable compromise for the general case (not a cache store).

  We could still do well to patch the MemoryStore to do one of the following:

  • Marshal everything (just like the file store), but this takes an unknown performance hit.
  • Marshal when not caching classes, to allow testing caching stuff in dev mode.
  • Make MemoryStore a null cache when reloading classes is enabled.
  • Refuse to use MemoryStore when class reloading is enabled.

  The first one may compromise the performance of the memory store, the second two feel like nasty nasty hacks, and I'm still not sure about the last one, feels too violent.

  Anyone fancy writing a patch for the vestigial classes to see how it feels? Anyone got any more thoughts on dealing with the memory cache?

• 

  Michael Koziarski March 9th, 2009 @ 04:23 AM

  The issue is slightly broader than just the cache stores though, you'll have the same issues if you used:

  $foo ||= Foo.first

  next request

  $foo.bar!

  Perhaps we could raise a particular exception, and document that exception to describe the different issues that can cause it?

• You flagged this item as spam.
  

  Scott Taylor March 9th, 2009 @ 06:13 AM

  Sure. That's the issue with using global vars, and class vars have exactly the same issue.

  I don't see what the big deal is here. Why aren't we just all using class level instance vars which only get initialized when they need to, like the following pastie:

  http://gist.github.com/76130

  This makes the thing lazy, and if the class magically goes away (via reloading through Module#remove_const), the data just gets reloaded the next time it is accessed.

  Am I missing something here?

  On another note, I like the exception idea, but I don't think there is any way you are going to be able to track absolutely everything that an object maybe holding on to, with the excepting of going to mod_ruby or invoking GC#each_object.

• 

  Michael Koziarski March 9th, 2009 @ 07:13 AM

  Class variables behave fine (with respect to this bug) if they're class vars on active record subclasses. e.g.

  class User < AR::Base cattr_accessor :most_popular end

  That's why making MemoryStore marshall the objects would be a reasonable fix as beyond caching there's not many cases which can't be more easily solved by slightly changing the design.

  As for the exception, I was simply thinking about overriding method_missing on the AR::Base subclass when we're calling undef_method to just raise the exception immediately, irrespective of the method or arguments.

• You flagged this item as spam.
  

  Shak April 6th, 2009 @ 04:22 PM

  So what has become the conventional way of dealing with this issue? To use Marshall? If so, does that imply having to create de/serializers too?

  Apologies, I'm new (but have found myself hitting this problem)!

• You flagged this item as spam.
  

  C. Bedard April 6th, 2009 @ 06:24 PM

  Just to make it clear for people who encounter the exception and end up reading about it here:

  • The exception is raised when ActiveRecord::Base#skip_time_zone_conversion_for_attributes is being evaluated, but that is only a symptom, so fixes that address only that specific attribute do not really address the real issue.

  The actual issue is caused by ActiveSupport::Dependencies not reloading classes declared in files that are explicitly required. This means you should avoid explicitly require'ing files and make your dependencies automatic, by naming and placing the files so that the Dependencies module will load them automatically.

  As for persistence of AR objects, as Tom Lea explained, the issue is really the fact that classes are redefined on each request, as opposed to having the same classes being reloaded on each request. A persisted object will always refer to the initial Class instance it was created with, whereas objects created on subsequent requests will refer to new classes (even if they have the same name).

  That being said, if I had to persist AR objects, I would stick to a persistence mechanism that only serializes its scalar (or rather: literal) attributes (i.e. String, Fixnum, etc.) and which assigns those values to a new AR instance that gets reinitialized on each request, but I would never hold on to an actual Class reference.

  I have to admit I am not familiar Marshall or MemStore and maybe that's the way those work. If that's the case, then it should pose no problem to use them for persistence.

• You flagged this item as spam.
  

  Tom Lea April 6th, 2009 @ 08:24 PM

  @Shak, @C. Bedard If you must store between requests, Marshal it. If you would simply prefer it to be there between requests, stash it in your controller's class, or the model's class, these will be reloaded anyway. If you are using a cache store, and are hitting this issue, just don't use the memory store.

  If you must marshal, don't worry, it's easy, everything works by 'magic' 99% of the time. Juts call Marshal.dump and Marshal.load as needed. Serialisation is handled for you, no need to define {,de}serializers, this is ruby, it uses its magic for good, not evil.

• You flagged this item as spam.
  

  Shak April 6th, 2009 @ 08:48 PM

  Thanks for the tips - I don't want to turn this into a support thread but just for completion: I'm trying to persist a semi static look up table created from an RPC call - a prime candidate for caching using Rails.cache. For the moment I'm using memstore (this may change to memcached later).

  Since my extended instance methods (the lookup is a hash of tableless AR objects) are being stripped I've marshalled (without any fancy coding) them and it all seems to be working rather well. Thanks for the pointers.

• You flagged this item as spam.
  

  jmoses April 8th, 2009 @ 12:34 AM

  Note that you also see this error if you relate an AR model in a plugin to one that is not in a plugin. It's bad practice, yes, but it exposes this error condition as well.

• You flagged this item as spam.
  

  maschnitz (at yahoo) April 10th, 2009 @ 09:52 PM

  • Title changed from “AR::Base should not be nuking it's children, just because it lost interest” to “AR::Base should not be nuking its children, just because it lost interest”

  I'd just like to note that this bug is really prominent on Metals. Class data members et al collide into this bug and create a lot of hurt. (If I follow what's happening correctly, that is...)

  Without workarounds, 2.3.2 Metals that hang on to things between requests are very hard to use in development mode as a result. You end up with some awkward calling conventions and/or a lot of Marshall stuff.

  Please, Rails gods, find a fix for this. :)

• You flagged this item as spam.
  

  Tom Lea April 11th, 2009 @ 04:59 AM

  Just tried the patch against 2.3, and I can't seem to find any evidence of a difference in memory leakynes anymore.

  Could someone (ideally @C. Bedard with he same app) try the patch against master to confirm, or prove me stupid? The existing patch applies clean.

  Thanks

  p.s. Thanks for the grammar fix maschnitz☺

• You flagged this item as spam.
  

  Courtenay August 13th, 2009 @ 01:22 AM

  I saw this bug in an app that required some models from an initializer. The second page load would die. This patch kinda-sorta fixed it, but if anyone's seeing this bug and can't figure out why, check your initializers for a require 'user'

• You flagged this item as spam.
  

  mail2lf (at gmail) August 13th, 2009 @ 05:24 PM

  Guys, don't know if it helps, but we just found the similar issue to occur.

  Having patched our Rails with the stuff you provided, we ran into the similar issue: we did something like

  user.level.bar,

  and sucked loudly, because belongs_to association (level on user) also used some class_inheritable_accessor (for scoping) and it also turned to become nil on and after 2nd FCKNG request.

  The hint there is that our model, user, actually subclasses the model stored in the plugin. Do you have any hint on this?

• You flagged this item as spam.
  

  mail2lf (at gmail) August 14th, 2009 @ 01:51 PM

  Never mind the previous comment, found require in the environment.rb. Sorry for senseless note.

• You flagged this item as spam.
  

  Josh Delsman September 8th, 2009 @ 07:02 PM

  I'm facing a similar issue, and I think I may have found a use case that may help.

  In two separate projects I'm working on, I have added an attr_accessor to the model for virtual attributes. The first is data and the second is number. Here are the implementations simplified:

  class Forecast < ActiveRecord::Base
    attr_accessor :data
  end
  
  class CreditCardProfile < ActiveRecord::Base
    attr_accessor :number
  end
  

  There are used virtually when the object is initialised to hold data which will not be present when the object is saved to the database. In both cases, using instance variables such as data and data= have resulted in the following error:

  >> credit_card_profile.save
  NoMethodError: You have a nil object when you didn't expect it!
  You might have expected an instance of Array.
  The error occurred while evaluating nil.include?
      from /Library/Ruby/Gems/1.8/gems/activerecord-2.3.3/lib/active_record/attribute_methods.rb:142:in `create_time_zone_conversion_attribute?'
      from /Library/Ruby/Gems/1.8/gems/activerecord-2.3.3/lib/active_record/attribute_methods.rb:75:in `define_attribute_methods'
      from /Library/Ruby/Gems/1.8/gems/activerecord-2.3.3/lib/active_record/attribute_methods.rb:71:in `each'
      from /Library/Ruby/Gems/1.8/gems/activerecord-2.3.3/lib/active_record/attribute_methods.rb:71:in `define_attribute_methods'
      from /Library/Ruby/Gems/1.8/gems/activerecord-2.3.3/lib/active_record/attribute_methods.rb:242:in `method_missing'
      from /Library/Ruby/Gems/1.8/gems/activerecord-2.3.3/lib/active_record/transactions.rb:206:in `rollback_active_record_state!'
      from /Library/Ruby/Gems/1.8/gems/activerecord-2.3.3/lib/active_record/transactions.rb:196:in `save'
      from (irb):98
  

  It looks as though this only affects models with virtual attributes such as these. Looking at line 142 for attribute_methods.rb, since [:datestamp, :timestamp] are already defined, I think its choking around the skip_time_zone_conversion_for_attributes method.

  I hope this helps!

• You flagged this item as spam.
  

  Kris Leech September 23rd, 2009 @ 07:08 PM

  • Assigned user cleared.

  I ran in to this problem when moving some models in to an 'engine' plugin. It occurs when I try and access a 'belongs_to' association in either the view or console.

  (eg. @document.author.name)

  /app/model/user.rb

  class User
  has_many :documents end

  /app/vendor/qwerty/app/models/document.rb

  class Document
  belongs_to :author, :class_name => 'User', :foreign_key => 'author_id' end

• You flagged this item as spam.
  

  Sai Emrys October 22nd, 2009 @ 05:13 AM

  I have this issue when using acts_as_versioned in rails 2.3.4 (also previously in 2.3.2).

  This works:
  Profile.first.updater.login
  This blows up on create_time_zone_conversion_attribute?:
  Profile.first.versions.first.updater.login

  Note that the version actually returns a Profile::Version, not Profile, which is a subclass of Profile.

  Also, Profile.first.versions.first.updater returns just fine AND (with inspect) shows the 'login' field normally. But somehow calling .login on it breaks.

  I added the config.active_record.default_timezone line and that no longer happens... on the first hit of the server. Instead, it gives this error:

  time_ago_in_words Profile.first.version.first.updated_at
  => undefined method abs' for Sat May 23 09:15:57 -0700 1970:Time @ vendor/rails/actionpack/lib/action_view/helpers/date_helper.rb:62:indistance_of_time_in_words'

  Note that Profile.first.version.first.updated_at appears to work fine in all cases.

  On subsequent hits, the create_time_zone_conversion_attribute? error happens again.

  This behavior is identical between dev and production mode, and my cache_classes settings are normal (off for dev, on for prod).

• You flagged this item as spam.
  

  Jose Fernandez March 6th, 2010 @ 08:00 PM

  Are there any updates on this issue? or plans on doing something about it?

• You flagged this item as spam.
  

  Henrique March 17th, 2010 @ 10:22 PM

  My turn. Lost 3 hours because this bug. It would be nice if at least a better error message could be presented.

• You flagged this item as spam.
  

  The Doctor What March 27th, 2010 @ 04:17 AM

  I also would like to know what the status is. At the moment, setting config.cache_classes = false is useless because you get random weird crashes that are impossible to track down; I can't tell if it is my bug or rails being buggy.

• 

  Jeremy Kemper May 4th, 2010 @ 06:48 PM

  • Milestone changed from “2.x” to “3.x”

  [bulk edit]

• You flagged this item as spam.
  

  Nicholas Faiz June 30th, 2010 @ 07:09 AM

  • Importance changed from “” to “”

  We've hit this error a number of times, in various forms. If you relate a model to anything in a plugin, in a module in lib, etc., you hit problems, under config.cache_classes = false, because Rails seems to unload it.

  I wrote a piece of Rack middleware which functions as a class loader, and can sit in development.rb and force Rails to reload these libraries for each request, and it removes the problem:

  @@@ Ruby

  class ClassLoader
  def initialize(app)

  @app = app
  

  end

  def call(env)

  load 'folder_entry.rb' unless defined? FolderEntry
  load 'cms.rb' unless defined? Cms
  load 'folder.rb' unless defined? Folder
  load 'cms_page.rb' unless defined? CmsPage    
  load 'article.rb' unless defined? Article   
  @app.call(env)
  

  end end

  config.middleware.use("ClassLoader")

  
  Note, you need to ensure that you only load the file if Rails has forgotten about it. If leave out the defined? check you'll reload files and Rails will lose state (especially with callbacks).
  
  This is a fix we've been using recently in development, and seems to be working.
  

• You flagged this item as spam.
  

  Nicholas Faiz June 30th, 2010 @ 07:13 AM

  Sorry about the poor formatting - the classloader is available from this gist at github: http://gist.github.com/439980

• 

  Jeremy Kemper July 1st, 2010 @ 06:58 PM

  • State changed from “open” to “resolved”

  This was fixed in http://github.com/rails/rails/commit/033e0a041f10ef4d4aa8ebb576560d... but it introduced a memory leak: #4183

• 

  José Valim July 1st, 2010 @ 07:05 PM

  This was fixed on master. We are investigating the memory leak.

• You flagged this item as spam.
  

  tribalvibes October 6th, 2010 @ 05:59 AM

  • Tag changed from “activerecord, bug, dependencies, reload” to “activerecord, associations, bug, dependencies, reload”

  @koz Yes, good idea about having method_missing spit out an exception with a detailed treatise and link to this ticket. That would have saved countless infuriating hours in dev. I would much rather have to restart the dev server every n requests due to some modest memory bloat (who cares, it's dev) vs. guaranteed have to restart it on every request or run with cache classes = true and restart on every change.

  Everything we do is based on rapid iteration and this situation and related const_get issues essentially completely negate the dynamic language value of ruby. Might as well have to run a compiler and linker between iterations! Or may I suggest instead of cache_classes config option, how about a leak_memory option with a default true for dev. Not being facetious, that would be a much better practical solution for our time and usability of the framework.

  There is so much red herring noise on this and the related threads that I think the core devs have lost sight of the core problems here. For us it manifests primarily as association attributes disappearing (and throwing nil related errors) between requests. We have models that use modest polymorphism and STI across multiple modules and referenced in associations. We are not intentionally caching or preserving any objects across requests except via exernal serialised stores. I say intentionally because I didn't plan on needing to learn the source internals of associations in order to use them effectively for app development. So, you could cross reference this ticket with https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets....

  The entire dynamic loading/reloading core needs to be revisited. The priorities are very simple. 1) Provide a stable repeatable dynamic dev environment that minimizes iteration times for any code changes. Edit, reload, repeat without server restarts. 2) The same code works the same way in production, only faster.

  Perhaps this work has all been done in Rails 3 but unfortunately we're stuck with a large legacy app and it's going to be a while before we can get there. So, that this issue was not fixed in 2.3 or 2.3.5 or 2.3.8 is very disconcerting to me. I urge you to roll whatever backports are necessary to put this into 2.3.x. Thanks to everyone who started #1290 where the symptom was the time zone as that lead to this discussion otherwise we likely would still be searching.

• You flagged this item as spam.
  

  Chuck Hoffman October 28th, 2010 @ 10:46 PM

  OK, my turn to chime in on this. We have a CMS-like system on which will build sites for numerous clients, so we make heavy use of plugins and Desert to extend the system where individual clients need custom functionality. We see this bug come up anywhere a model in one of these plugins has an association to one that's defined in the core system. So now we have to keep cache_classes=true on our dev/staging environment which is driving our front-end developers/designers crazy having to constantly dink with the tmp/restart.txt file.

  This thread has helped me a lot to understand the problem, but has so much in it now though, that I don't know what fix or patch to try any more. We're using 2.3.8. I tried the middleware class-loader approach but it seems not to play nice with Desert (OTOH, I'm not sure what exactly Desert is needed for that we don't get from Rails itself)

  Since the top of this thread claims a date of 10/25/10, is the patch linked to there the one I should try?

• You flagged this item as spam.
  

  Tom Lea October 29th, 2010 @ 11:37 AM

  The timestamp is a lie.

  But the patch will probably still work (not tried it recently, my focus moved on).

  You will find that any of the classes that were exploding will behave inconsistently instead. Last time I tried on a recent rails 2.3 the memory leak was gone, so go for it!

• You flagged this item as spam.
  

  Chuck Hoffman October 30th, 2010 @ 05:59 PM

  "Behave inconsistently" eh? :D Is it one of those deals where, if you edit the class's source code, the changes don't get loaded in?

• You flagged this item as spam.
  

  Tom Lea October 30th, 2010 @ 10:10 PM

  Weirder, you would have two versions loaded at once. Your plugin would reference the old version (via it's associations) and the rest of your app would see the new one. Most of the time this has little consequence, but you may need to restart your app when things get strange!

• You flagged this item as spam.
  

  Chuck Hoffman October 31st, 2010 @ 01:49 AM

  Ah. Not too bad I guess, I'm usually wither working on a plugin or on the main app, rarely both at the same time. I'll give it try.

• 

  Ryan Bigg November 8th, 2010 @ 01:48 AM

  • Tag cleared.

  Automatic cleanup of spam.

• You flagged this item as spam.
  

  csnk May 18th, 2011 @ 08:25 AM

  We are the professional uniforms manufacturer, uniforms supplier, uniforms factory, custom uniforms.

• You flagged this item as spam.
  

  learn italian online May 19th, 2011 @ 04:24 AM

  Nine state legislators from the Mexican State Ray Ban Jackie Ohh
  Ray Ban 3025
  Ray Ban 2140 of Sonora traveled to Tucson to complain about Arizona's new employer crackdown on illegal’s from Mexico. It seems that many Mexican illegal’s are returning to their hometowns and the officials in the Sonora state government are ticked off. A delegation of nine state legislators from Sonora was in Tucson on Tuesday to state that Arizona's new Employer Sanctions Law will have a devastating effect on the Mexican state.