Temporary fix for memory leak #170 #1571

Closed
wants to merge 1 commit into
from

Projects

None yet

3 participants

@DawidJanczak

Hello.

I've been trying to figure the memory leak in issue #170 out for recent few days and I think I know what may cause it.

First, a small disclaimer. Since I'm just learning Rails I have not been testing it in a real application environment, but instead used memory_spec test.
I would appreciate it if someone could test it in a real Rails application's production environment.

Now, to the point. There are two resource dependencies that somehow cause GC not to sweep the Resource objects after their respective Controllers are unregistered:

  1. Circular dependency between controller and its resource.
  2. The ResourceDSL config instance variable when registering resource with a block (e.g. Comments). I think this might be caused by the closure nature of Ruby blocks.

To fix the first issue I had to reset the circular dependency in the controller after its constant is unregistered.
Unfortunately to fix the second issue I had to introduce another circular dependency between Resource and ResourceDSL instances in order to remove it later.
Both of these are implemented in private clear_resource_bindings method in Namespace class.

These fixes are far from perfect to say the least - they break encapsulation and introduce another unneeded dependency.
(Un)Fortunately I am leaving for vacation soon, so this is all the work I can do on it for now.

Hopefully someone with deeper AA understanding will have some better ideas on how to fix these two situations :)

@DawidJanczak DawidJanczak Temporary fix for memory leak (issue 170).
Memory leak occurs when cleaning resources. Apart from unregistering
controller two resource references need to be cleaned:
1. circular dependency in controller
2. reference to resource in ResourceDSL instance
5548c54
@travisbot

This pull request passes (merged 5548c54 into 3130933).

@jpmckinney

Nice! I hope this can get merged soon. I would remove the comments:

#pending

# Workaround for memory leak.

# This instance is remembered in resource object in order to be removed
# when resource will be released (see issue 170 - memory leak).

and I would move the body of clear_resource_bindings into unload_resources!

@jpmckinney jpmckinney added a commit that referenced this pull request Aug 22, 2012
@jpmckinney jpmckinney tweak memory leak fix from #1571 e7001eb
@jpmckinney

Merged with mentioned changes.

@jpmckinney jpmckinney closed this Aug 22, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment