Skip to content

Commit 9b6fc10

Browse files
committed
Add AsyncActor state change callback
1 parent 0ff700a commit 9b6fc10

6 files changed

Lines changed: 110 additions & 32 deletions

File tree

google-cloud-debugger/lib/google/cloud/debugger/agent.rb

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,6 @@ def start
142142
# Once Debugger Agent is stopped, it cannot be started again.
143143
#
144144
def stop
145-
tracer.stop
146145
transmitter.stop
147146
async_stop
148147
end
@@ -171,10 +170,13 @@ def run_backgrounder
171170
end
172171

173172
##
174-
# @private Callback function to be invoked when the agent actor is about
175-
# to be stopped.
176-
def cleanup_callback
177-
tracer.stop
173+
# @private Callback function when the async actor thread state changes
174+
def on_async_state_change
175+
if async_running?
176+
tracer.start
177+
else
178+
tracer.stop
179+
end
178180
end
179181

180182
private
@@ -206,13 +208,6 @@ def ensure_debuggee_registration
206208
registration_result
207209
end
208210

209-
##
210-
# @private Override the #backgrounder_stoppable? method from AsyncActor
211-
# module. The actor can be stopped unconditionally.
212-
def backgrounder_stoppable?
213-
true
214-
end
215-
216211
##
217212
# @private Create a default logger
218213
def default_logger

google-cloud-debugger/lib/google/cloud/debugger/transmitter.rb

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -103,11 +103,9 @@ def run_backgrounder
103103
end
104104

105105
##
106-
# @private Callback function to be invoked when the transmitter actor
107-
# is about to be stopped.
108-
def cleanup_callback
106+
# @private Callback function when the async actor thread state changes
107+
def on_async_state_change
109108
synchronize do
110-
async_stop
111109
@queue_resource.broadcast
112110
end
113111
end

google-cloud-debugger/test/google/cloud/debugger/agent_test.rb

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,11 @@
6262
mocked_tracer = Minitest::Mock.new
6363
mocked_tracer.expect :stop, nil
6464

65+
agent.async_start
66+
6567
agent.stub :transmitter, mocked_transmitter do
6668
agent.stub :tracer, mocked_tracer do
67-
agent.stub :async_stop, nil do
68-
agent.stop
69-
end
69+
agent.stop
7070
end
7171
end
7272

@@ -80,6 +80,8 @@
8080
mocked_tracer = Minitest::Mock.new
8181
mocked_tracer.expect :stop, nil
8282

83+
agent.async_start
84+
8385
agent.stub :tracer, mocked_tracer do
8486
agent.stop
8587
end

google-cloud-debugger/test/google/cloud/debugger/middleware_test.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ def app.call(_) end
4040
mocked_tracer.expect :start, nil
4141
mocked_tracer.expect :disable_traces_for_thread, nil
4242

43+
# Construct middleware
44+
middleware
45+
4346
debugger.agent.stub :tracer, mocked_tracer do
4447
middleware.call({})
4548
end

stackdriver-core/lib/stackdriver/core/async_actor.rb

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ def async_stop
7676
synchronize do
7777
if async_state != :stopped
7878
@async_state = :stopping
79+
async_state_change
7980
@lock_cond.broadcast
8081
true
8182
else
@@ -95,6 +96,7 @@ def async_suspend
9596
synchronize do
9697
if async_state == :running
9798
@async_state = :suspended
99+
async_state_change
98100
@lock_cond.broadcast
99101
true
100102
else
@@ -114,6 +116,7 @@ def async_resume
114116
synchronize do
115117
if async_state == :suspended
116118
@async_state = :running
119+
async_state_change
117120
@lock_cond.broadcast
118121
true
119122
else
@@ -235,12 +238,7 @@ def self.unregister_for_cleanup actor
235238
def self.run_cleanup
236239
@exit_lock.synchronize do
237240
if @cleanup_list
238-
until @cleanup_list.empty?
239-
actor = @cleanup_list.shift
240-
241-
actor.cleanup_callback if actor.respond_to? :cleanup_callback
242-
actor.async_stop!
243-
end
241+
@cleanup_list.shift.async_stop! until @cleanup_list.empty?
244242
end
245243
end
246244
end
@@ -300,6 +298,7 @@ def async_run_job
300298
end
301299
ensure
302300
@async_state = :stopped
301+
async_state_change
303302
end
304303

305304
##
@@ -317,6 +316,7 @@ def ensure_thread
317316
AsyncActor.unregister_for_cleanup self
318317
end
319318
@async_state = :running
319+
async_state_change
320320
end
321321
end
322322
end
@@ -332,14 +332,15 @@ def set_cleanup_options **kwargs
332332
end
333333

334334
##
335-
# Abstract method that the inheriting classes should implement.
336-
#
337-
# When the actor is being gracefully asked to stop, it will only change
338-
# the actor status from :stopping to :stopped if the result of this method
339-
# returns true.
335+
# @private Default backgrounder stop condition when asked to be stopped
336+
# gracefully. Called from #async_run_job when async actor state changes
337+
# to :stopping
340338
def backgrounder_stoppable?
341-
fail "#{self.class} class should override " \
342-
"#backgrounder_stoppable? method"
339+
true
340+
end
341+
342+
def async_state_change
343+
on_async_state_change if respond_to? :on_async_state_change
343344
end
344345
end
345346
end

stackdriver-core/test/stackdriver/core/async_actor_test.rb

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ def run_backgrounder
3333
def start_thread &block
3434
block.call
3535
end
36+
37+
def on_async_state_change
38+
39+
end
3640
end
3741

3842
let(:actor) { AsyncActorTest.new }
@@ -52,6 +56,21 @@ def start_thread &block
5256
thr.kill
5357
thr.join
5458
end
59+
60+
it "calls #on_async_state_change" do
61+
mocked_change = Minitest::Mock.new
62+
mocked_change.expect :call, nil
63+
64+
actor.instance_variable_set :@lock_cond, OpenStruct.new(broadcast: nil)
65+
66+
actor.stub :on_async_state_change, mocked_change do
67+
actor.stub :ensure_thread, nil do
68+
actor.async_stop
69+
end
70+
end
71+
72+
mocked_change.verify
73+
end
5574
end
5675

5776
describe "#async_stop" do
@@ -68,6 +87,21 @@ def start_thread &block
6887
stopping = actor.async_stop
6988
stopping.must_equal false
7089
end
90+
91+
it "calls #on_async_state_change" do
92+
mocked_change = Minitest::Mock.new
93+
mocked_change.expect :call, nil
94+
95+
actor.instance_variable_set :@lock_cond, OpenStruct.new(broadcast: nil)
96+
97+
actor.stub :on_async_state_change, mocked_change do
98+
actor.stub :ensure_thread, nil do
99+
actor.async_stop
100+
end
101+
end
102+
103+
mocked_change.verify
104+
end
71105
end
72106

73107
describe "#async_suspend" do
@@ -84,6 +118,21 @@ def start_thread &block
84118

85119
actor.async_stop
86120
end
121+
122+
it "calls #on_async_state_change" do
123+
mocked_change = Minitest::Mock.new
124+
mocked_change.expect :call, nil
125+
126+
actor.instance_variable_set :@lock_cond, OpenStruct.new(broadcast: nil)
127+
128+
actor.stub :on_async_state_change, mocked_change do
129+
actor.stub :ensure_thread, nil do
130+
actor.async_stop
131+
end
132+
end
133+
134+
mocked_change.verify
135+
end
87136
end
88137

89138
describe "#async_resume" do
@@ -102,6 +151,21 @@ def start_thread &block
102151

103152
actor.async_stop
104153
end
154+
155+
it "calls #on_async_state_change" do
156+
mocked_change = Minitest::Mock.new
157+
mocked_change.expect :call, nil
158+
159+
actor.instance_variable_set :@lock_cond, OpenStruct.new(broadcast: nil)
160+
161+
actor.stub :on_async_state_change, mocked_change do
162+
actor.stub :ensure_thread, nil do
163+
actor.async_stop
164+
end
165+
end
166+
167+
mocked_change.verify
168+
end
105169
end
106170

107171
describe "#async_running?" do
@@ -233,6 +297,21 @@ def start_thread &block
233297
stop.must_equal :forced
234298
actor.async_stopped?.must_equal true
235299
end
300+
301+
it "calls #on_async_state_change" do
302+
mocked_change = Minitest::Mock.new
303+
mocked_change.expect :call, nil
304+
305+
actor.instance_variable_set :@lock_cond, OpenStruct.new(broadcast: nil)
306+
307+
actor.stub :on_async_state_change, mocked_change do
308+
actor.stub :ensure_thread, nil do
309+
actor.async_stop
310+
end
311+
end
312+
313+
mocked_change.verify
314+
end
236315
end
237316

238317
describe ".register_for_cleanup" do

0 commit comments

Comments
 (0)