Skip to content
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

Implement Eq for Thread #29447

Closed
wants to merge 2 commits into from
Closed

Implement Eq for Thread #29447

wants to merge 2 commits into from

Conversation

remram44
Copy link
Contributor

Fixes #21507

This simply compares the Arcs in the Threads since there is one Inner struct per thread.

Will add tests if there is interest in this.

@rust-highfive
Copy link
Collaborator

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

@eefriedman
Copy link
Contributor

The idea and implementation seem reasonable.

@alexcrichton
Copy link
Member

I would personally prefer an implementation route along the lines #29457, which is to say using the system primitives over our own abstractions, but I believe that today both will always yield the same results.

Regardless, though, I'm gonna tag this with T-libs so it comes up during libs triage. Thanks for the PR @remram44!

@alexcrichton alexcrichton added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Oct 29, 2015
@alexcrichton
Copy link
Member

Oh wait that doesn't make sense as an implementation strategy, these only contain our own allocations, nevermind!

@alexcrichton
Copy link
Member

The libs team discussed this during triage today and there was quite a bit of discussion and we didn't quite reach a conclusion. Some thoughts we had were:

  • It may not be the case that we want to literally compare Thread structures but rather perhaps an opaque ThreadId. This opaque structure could then have all the necessary traits implemented.
  • We weren't quite sure what the concrete use case was here. We could come up with some theoretical ideas, but we couldn't think of something we'd actually use it for in practice, so do you have something in mind for this?
  • This is closely entangled with Add a numeric ID to threads #29448, so we'll probably want to coordinate among those two.

@alexcrichton
Copy link
Member

Closing due to inactivity, but feel free to resubmit with the comments on this and #29448 addressed!

bors added a commit that referenced this pull request Oct 10, 2016
Add ThreadId for comparing threads

This adds the capability to store and compare threads with the current calling thread via a new struct, `std::thread::ThreadId`. Addresses the need outlined in issue #21507.

This avoids the need to add any special checks to the existing thread structs and does not rely on the system to provide an identifier for a thread, since it seems that this approach is unreliable and undesirable. Instead, this simply uses a lazily-created, thread-local `usize` whose value is copied from a global atomic counter. The code should be simple enough that it should be as much reliable as the `#[thread_local]` attribute it uses (however much that is).

`ThreadId`s can be compared directly for equality and have copy semantics.

Also see these other attempts:
- #29457
- #29448
- #29447

And this in the RFC repo: rust-lang/rfcs#1435
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants