Skip to content

Commit 51e28dc

Browse files
committed
Remove child event tracking from AS::Subscriber
Previously ActiveSupport::Subscriber would manage its own events, keep them in a thread-local stack, and track "child" events of the same subscriber. I don't think this was particularly useful, since it only considered notifications delivered to the same subscriber, and I can't find evidence of this being used in the wild. I think this was intended to support tracing, but any uch tool would want to do this itself (at minimum in order to support multiple namespaces). Additionally, this has caused a few users to OOM in production environments, notably when queueing many jobs from another job.
1 parent 86fd8d0 commit 51e28dc

File tree

3 files changed

+7
-29
lines changed

3 files changed

+7
-29
lines changed

activesupport/lib/active_support/log_subscriber.rb

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -107,14 +107,10 @@ def logger
107107
LogSubscriber.logger
108108
end
109109

110-
def start(name, id, payload)
111-
super if logger
112-
end
113-
114-
def finish(name, id, payload)
110+
def call(event)
115111
super if logger
116112
rescue => e
117-
log_exception(name, e)
113+
log_exception(event.name, e)
118114
end
119115

120116
def publish_event(event)

activesupport/lib/active_support/subscriber.rb

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -126,38 +126,18 @@ def fetch_public_methods(subscriber, inherit_all)
126126
attr_reader :patterns # :nodoc:
127127

128128
def initialize
129-
@queue_key = [self.class.name, object_id].join "-"
130129
@patterns = {}
131130
super
132131
end
133132

134-
def start(name, id, payload)
135-
event = ActiveSupport::Notifications::Event.new(name, nil, nil, id, payload)
136-
event.start!
137-
parent = event_stack.last
138-
parent << event if parent
139-
140-
event_stack.push event
141-
end
142-
143-
def finish(name, id, payload)
144-
event = event_stack.pop
145-
event.finish!
146-
event.payload.merge!(payload)
147-
148-
method = name.split(".").first
133+
def call(event)
134+
method = event.name.split(".").first
149135
send(method, event)
150136
end
151137

152138
def publish_event(event) # :nodoc:
153139
method = event.name.split(".").first
154140
send(method, event)
155141
end
156-
157-
private
158-
def event_stack
159-
registry = ActiveSupport::IsolatedExecutionState[:active_support_subscriber_queue_registry] ||= {}
160-
registry[@queue_key] ||= []
161-
end
162142
end
163143
end

activesupport/test/log_subscriber_test.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,9 @@ def test_event_is_an_active_support_notifications_event
7777

7878
def test_event_attributes
7979
ActiveSupport::LogSubscriber.attach_to :my_log_subscriber, @log_subscriber
80-
instrument "some_event.my_log_subscriber"
80+
instrument "some_event.my_log_subscriber" do
81+
[] # Make an allocation
82+
end
8183
wait
8284
event = @log_subscriber.event
8385
if defined?(JRUBY_VERSION)

0 commit comments

Comments
 (0)