Small context leak

[schneems]

When you create a block, it retains references to current variables in scope. We are creating blocks while trapping signals here:

puma/lib/puma/cluster.rb

Lines 332 to 334 in d56ad84

	Signal.trap "SIGCHLD" do
	wakeup!
	end

. These blocks are never garbage collected so they will retain references indefinitely. This includes a reference to every thread currently running via Thread.list

I think this should be fine for 99% of the cases, since most threads already running at that point will continue to run. However if you created a thread on boot that did something insane like read in a gig of data to the thread context and then discard the thread, this would accidentally retain a reference to the thread and accidentally retain that data. It's an edge case for sure.

To fix we could make a new method def self.setup_traps(master) and we could pass in only the main object, though we would need to expose @options[:workers] or pass it in.

It's not critical, but I think we should close this gap to be safe. What do you think?

[evanphx]

Yeah, let’s definitely close this gap. We should probably do another review
of all the code and see if there are other places this might be popping up.
It’s possible this is the source of a memory issue.

On Fri, Oct 16, 2015 at 1:16 PM, Richard Schneeman <notifications@github.com

wrote:

When you create a block, it retains references to current variables in
scope. We are creating blocks while trapping signals here:

puma/lib/puma/cluster.rb

Lines 332 to 334 in d56ad84

	Signal.trap "SIGCHLD" do
	wakeup!
	end

.
These blocks are never garbage collected so they will retain references
indefinitely. This includes a reference to every thread currently running
via Thread.list

I think this should be fine for 99% of the cases, since most threads
already running at that point will continue to run. However if you created
a thread on boot that did something insane like read in a gig of data to
the thread context and then discard the thread, this would accidentally
retain a reference to the thread and accidentally retain that data. It's an
edge case for sure.

To fix we could make a new method def self.setup_traps(master) and we
could pass in only the main object, though we would need to expose
@options[:workers] or pass it in.

It's not critical, but I think we should close this gap to be safe. What
do you think?

—
Reply to this email directly or view it on GitHub
#798.

[schneems]

I was curious if we could write tooling around this. We would need to detect when a proc scope is created and what the local and instance variables are when it is created. It looks like we can get that from TracePoint:

TracePoint.new(:b_call) do |tp|
  puts tp.binding.local_variables.inspect

  puts self.instance_variables.inspect

  file_line = "#{tp.path}:#{tp.lineno}"
  puts file_line.inspect
end.enable

a = nil
@boo = "foo"

->() {}.call

Then we would need to know what block objects are retained. If we know the block objects being retained, we can work backwards find out what variables the blocks have reference to, and boom...we've built an automated context leak detector.

Unfortunately i've not figured out this second part. i.e. how do you ask Ruby if an object is retained?

You can use ObjectSpace to grab objects:

require 'objspace'
ObjectSpace.trace_object_allocations do

  a = Proc.new {}

  b = -> {}

  def foo(&block)
    return block
  end

  c = foo do
  end
end

ObjectSpace.each_object(Proc) do |obj|
  puts obj
end

However the ObjectSpace.trace_object_allocations block will trace all allocated objects, even if they're not referenced anymore. I'm going to be at Keep Ruby Weird all day tomorrow. I wanted to write down this as a note in case anyone else was interested in fiddling with Ruby inspection and seeing if they had any ideas.

[schneems]

💥 thanks!