diff --git a/README.markdown b/README.markdown index 49699183..c04db951 100644 --- a/README.markdown +++ b/README.markdown @@ -103,6 +103,30 @@ Rails.application.configure do end ``` +### config.web_console.max_sessions + +Web Console retains the most recent error sessions in memory so that the +in-browser REPL can be re-attached to a captured stack frame. Each retained +session pins all stack-frame `Binding` objects, which in turn keep the entire +lexical scope alive — including `Rails.application.routes`, the controller +instance, and everything those reference. In long-running development +processes this previously grew without bound, leaking a full route tree +generation per dev-mode exception. + +Sessions are now evicted in insertion order once the configured limit is +reached. The default keeps the **5** most recent sessions, which is enough +to keep the in-browser console interactable while keeping memory bounded. + +```ruby +Rails.application.configure do + # Keep the 20 most recent sessions instead of the default 5 + config.web_console.max_sessions = 20 + + # Or disable eviction entirely (legacy unbounded behaviour, not recommended) + config.web_console.max_sessions = nil +end +``` + ### config.web_console.template_paths If you want to style the console yourself, then you can place `style.css` at a diff --git a/lib/web_console/railtie.rb b/lib/web_console/railtie.rb index 9bc8fdc6..1c79d32c 100644 --- a/lib/web_console/railtie.rb +++ b/lib/web_console/railtie.rb @@ -81,6 +81,12 @@ def web_console_permissions end end + initializer "web_console.max_sessions" do + if config.web_console.key?(:max_sessions) + Session.max_sessions = config.web_console.max_sessions + end + end + initializer "i18n.load_path" do config.i18n.load_path.concat(Dir[File.expand_path("../locales/*.yml", __FILE__)]) end diff --git a/lib/web_console/session.rb b/lib/web_console/session.rb index 58c0e7ce..7c26f339 100644 --- a/lib/web_console/session.rb +++ b/lib/web_console/session.rb @@ -11,15 +11,32 @@ module WebConsole # error pages only, as currently, this is the only client that needs to do # that. class Session + # In-memory store of recent sessions, keyed by id. Bounded by + # +max_sessions+ to prevent unbounded growth in long-running development + # processes: every captured session retains stack-frame +Binding+ objects + # which pin the entire lexical scope (including +Rails.application.routes+ + # via the controller +self+), so leaving the store unbounded leaks a full + # route tree generation per dev-mode exception. cattr_reader :inmemory_storage, default: {} + # Maximum number of concurrently retained sessions. When this many + # sessions are stored, inserting a new one evicts the oldest by + # insertion order (Ruby Hashes preserve insertion order). + # + # Set to a positive Integer to bound retention, or +nil+ to disable + # eviction entirely (legacy behaviour, leaks indefinitely). + cattr_accessor :max_sessions, default: 5 + + INMEMORY_STORAGE_MUTEX = Mutex.new + private_constant :INMEMORY_STORAGE_MUTEX + class << self # Finds a persisted session in memory by its id. # # Returns a persisted session if found in memory. # Raises NotFound error unless found in memory. def find(id) - inmemory_storage[id] + INMEMORY_STORAGE_MUTEX.synchronize { inmemory_storage[id] } end # Create a Session from an binding or exception in a storage. @@ -74,7 +91,18 @@ def context(objpath) private def store_into_memory - inmemory_storage[id] = self + INMEMORY_STORAGE_MUTEX.synchronize do + inmemory_storage[id] = self + evict_oldest_sessions + end + end + + def evict_oldest_sessions + limit = self.class.max_sessions + return unless limit && limit > 0 + + excess = inmemory_storage.size - limit + excess.times { inmemory_storage.shift } if excess > 0 end end end diff --git a/test/leak/reproduce_session_leak.rb b/test/leak/reproduce_session_leak.rb new file mode 100644 index 00000000..f40a0a3b --- /dev/null +++ b/test/leak/reproduce_session_leak.rb @@ -0,0 +1,99 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +# Reproduces the unbounded memory leak in WebConsole::Session.inmemory_storage. +# +# Each dev-mode exception captured by web-console creates a Session that +# retains stack-frame Binding objects. Bindings hold the full lexical scope, +# which in a Rails app pins `Rails.application.routes` -> the entire +# Journey route tree. Because the storage Hash has no eviction, every +# captured exception leaks a fresh RouteSet generation. +# +# The script simulates this without booting Rails: it builds a synthetic +# "route tree" payload, then captures a Binding that closes over it inside +# many short-lived contexts. The growth of `Session.inmemory_storage.size` +# and process RSS demonstrates the leak directly. +# +# Usage: +# bundle exec ruby test/leak/reproduce_session_leak.rb +# N_ERRORS=200 bundle exec ruby test/leak/reproduce_session_leak.rb +# MAX_SESSIONS=10 bundle exec ruby test/leak/reproduce_session_leak.rb +# +# Set MAX_SESSIONS=nil to opt back into the legacy unbounded behaviour. + +require "objspace" + +# Run from the gem root so we exercise THIS checkout, not an installed copy. +$LOAD_PATH.unshift File.expand_path("../../lib", __dir__) +require "active_support" +require "active_support/core_ext/class/attribute_accessors" +require "web_console/exception_mapper" +require "web_console/evaluator" +require "web_console/context" +require "web_console/session" + +# Per-iteration "RouteSet stand-in": a deeply-nested object graph sized to +# resemble what a real Rails::Engine::LazyRouteSet retains. Each FakeRouteSet +# instance allocates ~5 MB of heap so the leak is unmistakable in `ps`. +class FakeRouteSet + ROUTE_PAYLOAD_SIZE = 50_000 + + def initialize(generation) + @generation = generation + @routes = Array.new(ROUTE_PAYLOAD_SIZE) { |i| { gen: generation, id: i, path: "/r/#{generation}/#{i}" } } + end +end + +def rss_mb + File.read("/proc/self/status")[/VmRSS:\s+(\d+)/, 1].to_i / 1024 +rescue Errno::ENOENT + -1 # non-Linux; skip +end + +def emit(label, sessions, count, rss) + puts format("%-22s sessions=%-4d errors_seen=%-4d rss=%4d MB live_objects=%d", + label, sessions, count, rss, ObjectSpace.count_objects[:TOTAL]) +end + +# Apply config from environment. +case ENV["MAX_SESSIONS"] +when nil then # use the default declared in Session +when "nil", "" then WebConsole::Session.max_sessions = nil +else WebConsole::Session.max_sessions = Integer(ENV["MAX_SESSIONS"]) +end + +n_errors = Integer(ENV.fetch("N_ERRORS", 200)) + +puts "WebConsole::Session.max_sessions = #{WebConsole::Session.max_sessions.inspect}" +puts "Simulating #{n_errors} captured dev-mode exceptions..." +puts + +WebConsole::Session.inmemory_storage.clear +GC.start +emit("baseline", WebConsole::Session.inmemory_storage.size, 0, rss_mb) + +n_errors.times do |i| + # Each iteration creates a fresh "route tree" and captures a Binding that + # closes over it from inside a method frame -- exactly the shape of a real + # exception traceback through a Rails controller action. + route_set = FakeRouteSet.new(i) + capture = ->(binding_to_save) { WebConsole::Session.new([[binding_to_save]]) } + + # Force the closure to actually reference route_set by reading it inside the + # block. Otherwise the Ruby compiler can elide the capture. + _ = route_set + capture.call(binding) + + if (i + 1) % (n_errors / 10).clamp(1, 1_000_000) == 0 + GC.start + emit("after #{i + 1}", WebConsole::Session.inmemory_storage.size, i + 1, rss_mb) + end +end + +GC.start +puts +emit("final", WebConsole::Session.inmemory_storage.size, n_errors, rss_mb) +puts +puts "If max_sessions is bounded, `sessions` should plateau at the limit and" +puts "RSS should stay flat. With max_sessions = nil, `sessions` keeps growing" +puts "and RSS climbs roughly linearly with N_ERRORS." diff --git a/test/web_console/session_test.rb b/test/web_console/session_test.rb index 2b2d4313..5edb2506 100644 --- a/test/web_console/session_test.rb +++ b/test/web_console/session_test.rb @@ -106,5 +106,35 @@ def source_location assert_equal "=> WebConsole::SessionTest::ValueAwareError\n", session.eval("self") end + + test "evicts oldest sessions when max_sessions limit is reached" do + previous_max = Session.max_sessions + Session.max_sessions = 2 + + Session.inmemory_storage.clear + first = Session.new([[binding]]) + second = Session.new([[binding]]) + third = Session.new([[binding]]) + + assert_equal 2, Session.inmemory_storage.size + assert_nil Session.find(first.id) + assert_equal second, Session.find(second.id) + assert_equal third, Session.find(third.id) + ensure + Session.max_sessions = previous_max + end + + test "max_sessions = nil disables eviction" do + previous_max = Session.max_sessions + Session.max_sessions = nil + + Session.inmemory_storage.clear + sessions = Array.new(10) { Session.new([[binding]]) } + + assert_equal 10, Session.inmemory_storage.size + sessions.each { |s| assert_equal s, Session.find(s.id) } + ensure + Session.max_sessions = previous_max + end end end