Skip to content

export maxthreadid from Threads #57490

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

KristofferC
Copy link
Member

@KristofferC KristofferC commented Feb 21, 2025

maxthreadid is in the documentation and is referred to by other public docstrings so it should at least itself be public. Looking at what else is being exported I think this has the same "weight".

@KristofferC KristofferC added docs This change adds or pertains to documentation multithreading Base.Threads and related functionality backport 1.12 Change should be backported to release-1.12 labels Feb 21, 2025
This was referenced Mar 24, 2025
@KristofferC KristofferC mentioned this pull request Apr 4, 2025
51 tasks
@KristofferC KristofferC mentioned this pull request Apr 29, 2025
53 tasks
@KristofferC KristofferC mentioned this pull request May 9, 2025
58 tasks
@fingolfin
Copy link
Member

It is mentioned in the Threads.nthreads docs, and also listed in doc/src/base/multi-threading.md so this seems reasonable?

@vtjnash reacted with a 👎 perhaps you could elaborate what your concerns are?

@vtjnash
Copy link
Member

vtjnash commented May 13, 2025

There are almost no uses of this outside of internal scheduler code that are not subtle races and UB if you access this value

@fingolfin
Copy link
Member

OK, I find it a convincing argument that this should not be merged.

But then any docstrings of exported/public bindings referencing maxthreadid should likely also be adjusted to not do that anymore.

The only docstring I found referencing maxthreadid (besides its own docstring) is that of Threads.nthreads, but there it seems straight-forward to excise it... Did I miss anything?

@KristofferC KristofferC mentioned this pull request Jun 6, 2025
40 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.12 Change should be backported to release-1.12 docs This change adds or pertains to documentation multithreading Base.Threads and related functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants