Use local variable instead of instance variable to store the current thread client

[st0012]

In the current design, thread clients' relationship with the debugger session looks like this

<---------------------------------- Session ---------------------------------->
<----- event (TC-1) ----> <----- event (TC-2) -----> <----- event (TC-1) ----->

It always comes with an event and doesn't stay in the session after the event is processed.

But because it's currently stored in the session's instance variable @tc, we need to manage its state manually to make sure

1. it's assigned before an event is started
2. it's cleared after the event is finished

debug/lib/debug/session.rb

Lines 296 to 298 in 07c1a7c

	defwait_command_looptc
	@tc=tc

debug/lib/debug/session.rb

Lines 1344 to 1353 in 07c1a7c

	privatedefrestart_all_threads
	stopper=thread_stopper
	stopper.disableifstopper.enabled?
	waiting_thread_clients.each{|tc|
	nextif@tc == tc
	tc << :continue
	}
	@tc=nil
	end

Also, its availability is not guaranteed when processing different commands. For example, if the command is step, the @tc will be nil (reseted by restart_all_threads) until the next event sets its thread client. But if the command is bt, @tc will be remain unchanged.

<---------------------------------- Session -------------------------------->
<-------------- step cmd ------------> <-------------- bt cmd --------------> <--- @tc (TC-1) ---> <-- @tc (nil) --> <------------ @tc (TC-2) ------------>

Although none of these behaviors are causing issues, I think by making it a local variable we can:

• save the work to manage the ivar's state and avoid future bugs.
• make thread client's availability clear just by reading the code.

[st0012]

@ko1 I've tried various approaches but I can't find a way to make sure a thread is in the under control state, no matter it's running or sleeping. So I need to skip this case first to avoid randomly breaking tests.

[ko1]

In the current design, thread clients' relationship with the debugger session looks like this

<---------------------------------- Session ---------------------------------->
<----- event (TC-1) ----> <----- event (TC-2) -----> <----- event (TC-1) ----->

Sorry I can't understand the chart. How to read it?

[st0012]

<---------------------------------- Session ---------------------------------->
<----- event (TC-1) ----> <----- event (TC-2) -----> <----- event (TC-1) ----->

event means events from @q_evt.pop and the x-axis is time. I want to demonstrate that when the session handles several events in session_server_main, each event can come with different thread clients.

[ko1]

y-axis is time

So do Session (and finish it) and 3 events come at same time?

I'm not sure (1) why there are two events from TC-1 at the same time (2) why session is finished.

[st0012]

@ko1 that's a typo 🤦‍♂️ I meant x-axis is time, sorry.

[ko1]

Ah, I see.

Your opinion is, using local variables is cleaner than using ivars. Let me consider about that.

[ko1]

Now, I'm thinking current ivar approach is better because most of methods are limited run in session server and there is no danger to introduce wrong ivar (@tc). In other words, if wrong @tc is set and run, it should be bug.

This assertion (methods should run only in session server) should be guaranteed by the code.

[st0012]

@ko1 I know it'd be a bug if @tc is incorrectly set. The problem is, because it's an instance variable, it's not easy to track its status change and debug.

For example, restart_all_threads would set @tc = nil. So if you call a method that implicitly relies on @tc, like repl_add_breakpoint, after restart_all_threads is called, it'll raise NoMethodError.
This means that there are ordering dependencies between session helpers, like

# this would workrepl_add_breakpointstep_command# this would breakstep_commandrepl_add_breakpoint

I can see myself getting bitten by this kind of implicit ordering when developing features in the future. (I spotted this when experimenting with TUI support).

So the idea is that if we can make tc a local variable and pass it around as an explicit dependency of helpers, it'll be easy to spot future problems.