Skip to content
This repository has been archived by the owner on Sep 18, 2020. It is now read-only.

Try to do the same thing with finalizer #1

Merged
merged 6 commits into from
May 13, 2020
Merged

Conversation

schneems
Copy link
Owner

@schneems schneems commented Dec 2, 2016

No description provided.

trace = ObjectTrace.new(*args)

self.tracing_hash[trace.key] = trace
self.freed_hash[trace.key] = false

ObjectSpace.define_finalizer(args.first, -> (object_id) { LivingDead.freed_hash[object_id] = true })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to write a method that generates the lambda. This lambda will capture the environment and hold a reference to args, which I assume contains a reference to the object you want released?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. Related: would like to make a tool to make a context leak like this easier to find/debug if it's happening somewhere in a larger app. Working on a WeakRef solution, which is yet another thing I didn't realize existed.

@tenderlove
Copy link
Contributor

You might be able to simplify further by using a WeakRef:

require 'weakref'

obj = Object.new
ref = WeakRef.new obj
p ref.weakref_alive? # => true
obj = nil
GC.start
p ref.weakref_alive? # => nil

@tenderlove
Copy link
Contributor

Or even

require 'weakref'

class WeakRef
  def collected?
    !weakref_alive?
  end
end

obj = Object.new
ref = WeakRef.new obj
p ref.collected? # => false
obj = nil
GC.start
p ref.collected? # => true

@schneems
Copy link
Owner Author

schneems commented Dec 3, 2016

Thanks again for pointing out both finalizers and weakref. This implementation with weakref works great. Actually with Weakref, not even sure this library needs to exist.

Unfortunately I'm still having the same problems with GC. Need to call it multiple times and need to call puts, still no clue why.

Also this isn't as bulletproof as the original C-extension. When I run the tests in a loop I eventually get a failure:

Failures:

  1) LivingDead calling `singleton_class.instance_eval`  does not retain in a class
     Failure/Error: expect(out).to match("PASS")

       expected "FAIL: expected: 0, actual: 1\n" to match "PASS"
       Diff:
       @@ -1,2 +1,2 @@
       -PASS
       +FAIL: expected: 0, actual: 1
     # ./spec/living_dead/singleton_class_instance_eval_spec.rb:17:in `block (3 levels) in <top (required)>'

Finished in 10.67 seconds (files took 0.11974 seconds to load)
19 examples, 1 failure

Failed examples:

rspec ./spec/living_dead/singleton_class_instance_eval_spec.rb:15 # LivingDead calling `singleton_class.instance_eval`  does not retain in a class

/Users/richardschneeman/.rubies/ruby-2.3.3/bin/ruby -I/Users/richardschneeman/.gem/ruby/2.3.3/gems/rspec-core-3.5.4/lib:/Users/richardschneeman/.gem/ruby/2.3.3/gems/rspec-support-3.5.0/lib /Users/richardschneeman/.gem/ruby/2.3.3/gems/rspec-core-3.5.4/exe/rspec --pattern spec/\*\*\{,/\*/\*\*\}/\*_spec.rb failed

real	1m18.093s
user	1m7.182s
sys	0m8.836s
2.3.3  ~/documents/projects/living_dead (schneems/finalizer)
$ time while bundle exec rake spec; do :; done

@schneems
Copy link
Owner Author

schneems commented Dec 3, 2016

Actually scratch that comment about this being less bullet proof than master. I just got a failure there too:

  1) LivingDead calling `singleton_class.instance_eval`  does not retain in a class
     Failure/Error: expect(out).to match("PASS")

       expected "FAIL: expected: 0, actual: 1\n" to match "PASS"
       Diff:
       @@ -1,2 +1,2 @@
       -PASS
       +FAIL: expected: 0, actual: 1
     # ./spec/living_dead/singleton_class_instance_eval_spec.rb:17:in `block (3 levels) in <top (required)>'

Finished in 11.03 seconds (files took 0.12631 seconds to load)
19 examples, 1 failure

Failed examples:

rspec ./spec/living_dead/singleton_class_instance_eval_spec.rb:15 # LivingDead calling `singleton_class.instance_eval`  does not retain in a class

/Users/richardschneeman/.rubies/ruby-2.4.0-preview2/bin/ruby -I/Users/richardschneeman/.gem/ruby/2.4.0/gems/rspec-core-3.5.4/lib:/Users/richardschneeman/.gem/ruby/2.4.0/gems/rspec-support-3.5.0/lib /Users/richardschneeman/.gem/ruby/2.4.0/gems/rspec-core-3.5.4/exe/rspec --pattern spec/\*\*\{,/\*/\*\*\}/\*_spec.rb failed

real	1m43.005s
user	1m24.500s
sys	0m11.849s
2.4.0  ~/documents/projects/living_dead (master)
$ time while bundle exec rake spec; do :; done && say done

So it seems that there is some kind of bug, some of the time, that prevents objects from being properly freed even with my weird puts and 10.times { GC.start }.

@schneems
Copy link
Owner Author

I ported over schneems/heap_problem#1 to try to make this deterministic. It passes on my machine but fails on travis. I'm able to repro with the docker image locally.

@matthewd any ideas on the difference here between mac and linux? Different stack size? Maybe it's different in a container? Here's the most recent commit 59cfa83

schneems added 3 commits July 26, 2018 09:34
Instead of picking a magic number and we can create an object in memory that is not retained, we then trace the object and while we detect that it is in memory we repeatedly flush the stack and create new objects. Once the object is out of memory then we can determine that it’s been removed by GC. 

The theory here is that since the object we are tracing is more recent than the object that the user is tracing it will be more likely that GC will have cleared any unused objects before getting to the one we just created.

It’s still not perfect, but hopefully it’s better than it was previously.
@schneems
Copy link
Owner Author

Got an idea talking with @olivierlacan and i've got a new implementation pushed to this branch d61fe4c

The new idea is that we can be smarter about how many times we need to "flush" the stack by monitoring a known non-retained object.

I'm still not 100% sure that this "solves" the problem, but it is light years beyond what I was doing. What do you think @matthewd @tenderlove @ko1 ?

@matthewd
Copy link

The theory here is that since the object we are tracing is more recent than the object that the user is tracing it will be more likely that GC will have cleared any unused objects before getting to the one we just created.

I don't believe that theory holds. If an object survives a plain GC.start, then somewhere in memory there is [a set of 8 bytes that looks like] a reference to it -- though that could be dead memory, so it's not necessarily a "real" reference.

Given that such a reference exists, the object's longevity is based on how long until that unused memory gets overwritten -- which has nothing to do with any other new objects that get created & destroyed in the meantime.

AFAICS, your while loop will spin until a natural [minor] GC is triggered, at which point the trace object will have been freed. It is (again, I believe) equivalent to an un-looped block_stack_flush(n); GC.start(full_mark: false) -- where n is an arbitrary value based on how many allocations it takes to hit the GC, unrelated to the true depth.

@schneems
Copy link
Owner Author

I was thinking that maybe the issue was incremental GC and not that something looked like a reference. But I guess I have no way of seeing if the object was unmarked or not unless I instrumented the GC code.

@schneems schneems merged commit 7512094 into master May 13, 2020
@schneems
Copy link
Owner Author

Still not "solved" but i'm cleaning my PRs. It's better than what's in master, but I'm not using this anywhere.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants