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

AsRawHandle and IntoRawHandle for JoinHandle #29461

Closed
wants to merge 2 commits into from

Conversation

retep998
Copy link
Member

This allows users to get the HANDLE of a spawned thread on Windows.
Ditto with pthread_t and non-Windows.

@retep998
Copy link
Member Author

r? @alexcrichton

@rust-highfive rust-highfive assigned alexcrichton and unassigned brson Oct 29, 2015
@rust-highfive
Copy link
Collaborator

r? @brson

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

@alexcrichton
Copy link
Member

Looks good to me! I would like to ensure feature parity between Unix and Windows, however, so I think that this would want to land in tandem with a lowering from JoinHandle<T> to &pthread_t as well.

@retep998
Copy link
Member Author

There's no (Into|As)Raw equivalent for pthreads so I'm not sure how such an API would look. I'm guessing new extension traits would have to be added, and if so what should they be called, (As|Into)RawPthread? Should they be added as unstable to begin with?

@alexcrichton
Copy link
Member

Yeah, although I wouldn't add a *RawPthread family of traits but rather just a JoinHandleExt trait with appropriate methods (as nothing else will ever implement this trait)

@retep998 retep998 force-pushed the handling-threads branch 4 times, most recently from 7ab5316 to 2cae9a3 Compare October 31, 2015 01:01
@retep998
Copy link
Member Author

Added unix implementation.


pub fn id(&self) -> &libc::pthread_t { &self.id }

pub fn into_id(self) -> libc::pthread_t { self.id }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this will want to cancel the destructor via mem::forget

@alexcrichton
Copy link
Member

Thanks! I'll flag this with T-libs so it comes up during triage.

@alexcrichton alexcrichton added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Nov 2, 2015
This allows users to get the HANDLE of a spawned thread on Windows

Signed-off-by: Peter Atashian <retep998@gmail.com>
@retep998 retep998 force-pushed the handling-threads branch 2 times, most recently from 7ea8c35 to bdbfe0c Compare November 6, 2015 16:56
@retep998
Copy link
Member Author

retep998 commented Nov 6, 2015

Addressed comments. Travis seems like it is way behind.

Signed-off-by: Peter Atashian <retep998@gmail.com>
@@ -15,6 +15,8 @@
#[stable(feature = "raw_ext", since = "1.1.0")] pub type uid_t = u32;
#[stable(feature = "raw_ext", since = "1.1.0")] pub type gid_t = u32;
#[stable(feature = "raw_ext", since = "1.1.0")] pub type pid_t = i32;
#[unstable(feature = "pthread_t", issue = "0")]
pub type pthread_t = usize;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually a little more involved, taking a look at the definition in liblibc:

$ git grep 'type pthread_t'
src/unix/bsd/mod.rs:pub type pthread_t = ::uintptr_t;
src/unix/notbsd/android/mod.rs:pub type pthread_t = c_long;
src/unix/notbsd/linux/mod.rs:pub type pthread_t = c_ulong;

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are c_long and c_ulong ever a different size than usize on Linux and Android?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In terms of what actually happens they're all the same size (I believe), but I think it's best to ensure that they're the same definition so the libstd pthread_t is interoperable with the libc pthread_t.

@alexcrichton
Copy link
Member

The libs team discussed this today during triage and we were definitely ok with the Windows impls but there are some other possible questions around the Unix ones (which I've now filed in the tracking issue). If you could update the tracking issue references then we felt this was good to go because the Unix aspects are still unstable.

@alexcrichton
Copy link
Member

Closing due to inactivity, but feel free to resubmit with comments addressed!

bors added a commit that referenced this pull request Dec 5, 2015
Allows a `HANDLE` to be extracted from a `JoinHandle` on Windows.
Allows a `pthread_t` to be extracted from a `JoinHandle` everywhere else.

Because #29461 was closed.

r? @alexcrichton
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.

4 participants