Show running thread in async registry #21776
Open
+104
−34
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jbajic
reviewed
May 20, 2025
arangod/AsyncRegistryServer/PrettyPrinter/src/asyncregistry/gdb_data.py
Outdated
Show resolved
Hide resolved
arangod/AsyncRegistryServer/PrettyPrinter/src/pretty-printer.py
Outdated
Show resolved
Hide resolved
goedderz
reviewed
May 20, 2025
lib/Containers/Concurrent/thread.h
Outdated
| @@ -36,8 +36,7 @@ struct ThreadId { | |||
| }; | |||
| template<typename Inspector> | |||
| auto inspect(Inspector& f, ThreadId& x) { | |||
| return f.object(x).fields(f.field("LWPID", x.kernel_id), | |||
| f.field("name", x.name())); | |||
| return f.object(x).fields(f.field("LWPID", x.kernel_id)); | |||
There was a problem hiding this comment.
Is there a situation where having the posix_id in addition to kernel_id might be helpful to know while we don't have the name?
There was a problem hiding this comment.
As far as I understood, the posix id does not provide more information, but it can act as a handle to do more stuff with it via the posix API, e.g. get the thread name yourself. The lwpid is just a low level thread id of the kernel. So perhaps it can be useful to additionally have the posix id as well. I'll add it back.
There was a problem hiding this comment.
It's fine by me without it, it really was an open question. The cases I can think of I'd like to see the LWPID, and can't think of something where I'd need the posix_id.
goedderz
approved these changes
May 20, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Arangod crashed after requesting the async registry, due to a bug: When reading the registry, every promise inside it requests the thread name of its owning thread in order to print that name. The problem is that this owning thread does not need to exist any more although the promise still exists, e.g. because the promise was suspended (so is currently not running at all) or resumed on another thread. So when requesting the thread name of a thread that does not exist any more, we crash.
Actually, the owning thread is not interesting for the user, the user just wants to see on which thread the promise is currently running (if it is running). Therefore, this PR adds a running_thread member to the promise and updates it when a state change occurs: if the state is running, then the promise has a running thread, otherwise the promise has no running thread.
The PR also updates the pretty printers of the async registry.