GdbServer: Rework sleeping #4208
Conversation
Sonicadvance1
force-pushed
the
gdbserver_rework_sleeping
branch
4 times, most recently
from
e1d8d38 to
0b2abf9
Compare
|
Removed the dependency on #4208 for this PR since this is independent. |
Sonicadvance1
force-pushed
the
gdbserver_rework_sleeping
branch
from
0b2abf9 to
19a92ab
Compare
Sonicadvance1
force-pushed
the
gdbserver_rework_sleeping
branch
2 times, most recently
from
08b7dca to
8621f25
Compare
This is enough to get GdbServer working for my simple test case again. Some things of note: - There is now a callback which signals to GdbServer when a thread is created. - Thread sleeping is now a single uint32_t futex word for knowing when a thread is sleeping and if we need to signal it to wake up. - GdbServer thread sleeping versus ThreadManager thread sleeping distinction has been removed. GdbServer now uses ThreadManager to put a thread to sleep. This still only gets the attach at startup scheme of gdbserver working. I haven't yet dived back in to getting arbitrary attach working. Primarily the work necessary here is actually getting thread creation at the start to stop and wait for gdbserver to attach. Then actually sleeping correctly while gdbserver is communicating with gdb.
Sonicadvance1
force-pushed
the
gdbserver_rework_sleeping
branch
from
7c9bbc5 to
f79cb4e
Compare
Sonicadvance1
force-pushed
the
gdbserver_rework_sleeping
branch
from
f79cb4e to
a0159e1
Compare
| class InspectableLatch final { | ||
| public: | ||
| bool Wait(struct timespec* Timeout = nullptr) { | ||
| LOGMAN_THROW_A_FMT(Mutex.load() != HAS_SIGNALED, "Stale signal mutex!"); |
There was a problem hiding this comment.
This line breaks pre-signaled latches, doesn't it?
There was a problem hiding this comment.
It specifically doesn't support pre-signaled latches. That's one of the two reasons why I implemented this.
There was a problem hiding this comment.
Why did you copy my docstring proposal if pre-signaling is not supported? The two are in direct contradiction as-is.
That's one of the two reasons why I implemented this.
You implemented InspectableLatch in order to be able to... not support something? Why?
(FTR I'd suggest not live-iterating the docstring any further until we find a suitable alternative, since I feel like I'm understanding increasingly less what this class is trying to achieve but GitHub makes going back to your older revisions a pain.)
| } | ||
|
|
||
| void NotifyOne() { | ||
| #if defined(ASSERTIONS_ENABLED) && ASSERTIONS_ENABLED |
There was a problem hiding this comment.
Another unnecessary preprocessor guard.
| @@ -81,8 +157,7 @@ struct ThreadStateObject : public FEXCore::Allocator::FEXAllocOperators { | |||
| std::atomic<SignalEvent> SignalReason {SignalEvent::Nothing}; | |||
|
|
|||
| // Thread pause handling | |||
| std::atomic_bool ThreadSleeping {false}; | |||
| FEXCore::InterruptableConditionVariable ThreadPaused; | |||
| InspectableLatch ThreadSleeping; | |||
There was a problem hiding this comment.
This seems to be a poor name, since ThreadSleeping.HasWaiter() can't be made sense of in isolation.
Something like ThreadWakeupLatch/ThreadResumeLatch/ThreadUnpauseLatch seem more reasonable: ThreadResumeLatch.HasWaiter() and ThreadResumeLatch.NotifyOne() are both clear without having to jump around the source.
This is enough to get GdbServer working for my simple test case again.
Some things of note:
created.
thread is sleeping and if we need to signal it to wake up.
distinction has been removed. GdbServer now uses ThreadManager to put
a thread to sleep.
This still only gets the attach at startup scheme of gdbserver working.
I haven't yet dived back in to getting arbitrary attach working.
Primarily the work necessary here is actually getting thread creation at
the start to stop and wait for gdbserver to attach. Then actually
sleeping correctly while gdbserver is communicating with gdb.