memory leak in RenderingContext

[catmando]

RenderingContext.render builds a buffer of rendered elements. For example:

DIV do
  SPAN { 'span 1'}; SPAN { 'span 2' }; SPAN { 'span 3' }
end

rendering the DIV adds the three SPANs to the DIVs rendering buffer.
As each render completes the buffer is packed up as a the children of the element being rendered, and the newly rendered element is pushed onto the parents buffer...
So we have a stack of these buffers.

The problem is nobody ever clears the top most rendering buffer, so after each rendering cycle all of the top most renders children remain in the buffer, and the buffer just gets longer and longer.

There is a bunch of complications between how the buffer is managed to allow for nodes to be pulled from the buffer, etc, so the fix that I propose is kind of ugly:

Just use a class instance variable to detect if its an outer most render, and if it is, then reset the buffer in the ensure block.

[catmando]

Here is a patch that I've been testing... suggest we just use as is for now:

module React
  class RenderingContext
    class << self
      alias pre_patch_render render
      def render(name, *args, &block)
        was_outer_most = !@not_outer_most
        @not_outer_most = true
        pre_patch_render(name, *args, &block)
      ensure
        @not_outer_most = @buffer = nil if was_outer_most
      end
    end
  end
end

well not the patch :-) just integrated into RenderingContext

[zetachang]

Is the outer_most render always happen from https://github.com/ruby-hyperloop/hyper-react/blob/master/lib/react/component.rb#L121 ?

If so, we could reset the buffer right after the render finished as following diff.

def _render_wrapper
  State.set_state_context_to(self, true) do
    element = React::RenderingContext.render(nil) { render || '' }
+   React::RenderingContext.instance_variable_set(:@buffer, nil)
    @waiting_on_resources =
      element.waiting_on_resources if element.respond_to? :waiting_on_resources
    element
  end
# rubocop:disable Lint/RescueException # we want to catch all exceptions regardless
rescue Exception => e
  # rubocop:enable Lint/RescueException
  self.class.process_exception(e, self)
end

[catmando]

Nope

[zetachang]

I apply the patch and see when is was_outer_most become true by running against our test suits, and it turns out the _render_wrapper we place at React::Component seems always to be the outer most. Am I missing anything?

My plan on this is to create a separate rendering context for every _render_wrapper and assign it to something like React::RenderingContext.current_context which may make further refactoring easier. Does it make sense?

[catmando]

Yes I think so