Page MenuHomePhabricator

[libc++] Add an extension to allow constructing std::thread from native handles
AbandonedPublic

Authored by ldionne on Nov 23 2020, 12:44 PM.

Details

Reviewers
None
Group Reviewers
Restricted Project
Summary

On some systems, creating threads with the standard std::thread constructor
is not convenient, or downright impossible. For example, some systems require
passing a stack size when creating a thread, but the std::thread constructor
does not allow for that.

This patch adds a non-standard extension to std::thread that makes it
constructible from the native_handle_type. This allows creating a thread
using the underlying system's machinery (e.g. in a factory function),
while retaining the rest of the std::thread API.

Diff Detail

Event Timeline

ldionne created this revision.Nov 23 2020, 12:44 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 23 2020, 12:44 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
ldionne requested review of this revision.Nov 23 2020, 12:44 PM

I'm interested to hear what folks like @EricWF, @jfb, @wash and @__simt__ have to say about this. This appears to be the easiest and most extensible way to allow creating threads on platforms that require stuff like a stack size. I read https://wg21.link/p2019, but I wasn't convinced by the reason why they did not push on this alternative.

I have concrete needs for this, and I think it would be a valuable experiment to make in libc++. I'd be OK with enabling this extension conditionally only, however I don't think the increased configuration complexity is worth it. Thoughts?

jfb added a subscriber: jwakely.Nov 23 2020, 2:06 PM

This is a sensible thing to support IMO. It's indeed weird to have native handle in other places, but not here.

What are the requirements on lifetime with this change? i.e. usually the thread owns the handle, but now it doesn't. Don't you need a bit to track "I don't own this" in the implementation? In particular, should the destructor act differently? Or is thread taking ownership at this point?

Check with @jwakely too, it would be better if libstdc++ had the same extension.

In D91992#2412458, @jfb wrote:

This is a sensible thing to support IMO. It's indeed weird to have native handle in other places, but not here.

What are the requirements on lifetime with this change? i.e. usually the thread owns the handle, but now it doesn't. Don't you need a bit to track "I don't own this" in the implementation? In particular, should the destructor act differently? Or is thread taking ownership at this point?

Conceptually, the std::thread owns the native handle after being constructed from one. So it would be more correct to use thread(native_handle_type&&), and then you'd construct a std::thread with std::thread t(std::move(handle));. However, since native handles are basically C structs or even plain integers, IDK whether it makes sense to do that. Maybe?

jfb added a comment.Nov 23 2020, 2:24 PM

This is a sensible thing to support IMO. It's indeed weird to have native handle in other places, but not here.

What are the requirements on lifetime with this change? i.e. usually the thread owns the handle, but now it doesn't. Don't you need a bit to track "I don't own this" in the implementation? In particular, should the destructor act differently? Or is thread taking ownership at this point?

Check with @jwakely too, it would be better if libstdc++ had the same extension.

In D91992#2412458, @jfb wrote:

This is a sensible thing to support IMO. It's indeed weird to have native handle in other places, but not here.

What are the requirements on lifetime with this change? i.e. usually the thread owns the handle, but now it doesn't. Don't you need a bit to track "I don't own this" in the implementation? In particular, should the destructor act differently? Or is thread taking ownership at this point?

Conceptually, the std::thread owns the native handle after being constructed from one. So it would be more correct to use thread(native_handle_type&&), and then you'd construct a std::thread with std::thread t(std::move(handle));. However, since native handles are basically C structs or even plain integers, IDK whether it makes sense to do that. Maybe?

Yeah that makes sense, but it's a bit surprising. More of a library design question at that point: do you want to have a move_from or take or whatever static member instead? So it's clear. I don't have a strong preference, it's just slightly surprising.

I'm not sold on this but I'm not deeply opposed either.

It's entirely possible for the C++ library to add code around the thread launch to ensure some invariants it has (X state is initialized, Y mode is enabled/disabled, Z mechanism is armed to track this thread exists/exits) and will not be ensured by going to a lower layer and just creating a thread there directly. That could be true even when the C++ library itself does that, and could give you the native handle that results from itself doing it.

Role-playing SG1 for a bit, I think if SG1 took on this extension it would either be optional/implementation-defined or it would have permission to throw if it can't adopt the thread (and on some platforms, potentially it would _always_ throw). That might result in some other ancillary design elements, like a constexpr way to query if adoption is supported at all and/or a way for the native launch to invoke the code that thread uses to ensure its invariants.

Also, applying this treatment to jthread is a bit less elegant again, which is annoying, but not impossible.

Ok, backing up: should libcxx have this extension?

As I said, I'm not opposed to it but a little while ago I was told that libcxx doesn't do extensions in the mainline. I don't remember what it was I had in mind, maybe it was atomic<>::wait() support for timeouts. At the time, the answer was to do this in a deployment fork, not mainline.

Do we want to do nice extensions? And if so, what's the basic litmus test for knowing we should do it?

This reminds me of an often-requested extension to std::fstream to allow it to take ownership of a FILE* or unix file descriptor. In libstdc++ we do support that, but only by using a non-standard filebuf type, __gnu_cxx::stdio_filebuf. It's not available for std::basic_filebuf. Doing that would be an alternative design for this (that is, a non-standard native_thread type that you'd use instead of std::thread). That wouldn't be interchangeable with std::thread though, so I can see why you want to extend the existing type.

I was going to mention that p2019 suggests a very different approach for platform-specific properties of the new thread, but I see you already covered that.

I agree with Olivier that this would have to be conditionally supported, but that's already implied by the use of native_handle_type (which isn't required to exist). For libstdc++, we don't currently do anything special that's specific to C++ when creating a new thread, so we could support this exactly the same way.

However, I'm concerned that this changes the result of things like is_constructible<thread, int> or is_constructible<thread, void*>. On GNU/Linux thread::native_handle_type is just unsigned long, on MinGW-w64 it's uintptr_t, so this would mean you can construct a std::thread with any integer. On other targets it's a void*. I'd be happier if the new constructor required a tag type to say "I know what I'm doing", e.g. thread(from_native_t, native_handle_type) so you'd say std::thread t(std::from_native, thr);. Or JF's suggestion of a static member function would also work: auto t = std::thread::from_native(thr);

For jthread constructing from a native handle isn't going to allow you to stop the thread, meaning it will hang in the destructor (or move assignment op). So there seems no point adding the same constructor to jthread. But you could allow construction from a native handle and a stop_source (with a precondition that stop_possible() is true). People could still shoot themselves in the foot by passing a stop source that is unconnected to the thread the handle refers to, but at least there's a possibility to get it right.

ldionne abandoned this revision.Nov 27 2020, 9:15 AM

Ok, backing up: should libcxx have this extension?

As I said, I'm not opposed to it but a little while ago I was told that libcxx doesn't do extensions in the mainline. I don't remember what it was I had in mind, maybe it was atomic<>::wait() support for timeouts. At the time, the answer was to do this in a deployment fork, not mainline.

You're right, actually I'm probably the one who said that. As we said offline, I think it might make it more reasonable to support an extension if there's a proposed paper for it, but this isn't the case here. It's fine to carry extensions downstream, and that's what I'll do. Thanks for reminding me :).

This reminds me of an often-requested extension to std::fstream to allow it to take ownership of a FILE* or unix file descriptor. In libstdc++ we do support that, but only by using a non-standard filebuf type, __gnu_cxx::stdio_filebuf. It's not available for std::basic_filebuf. Doing that would be an alternative design for this (that is, a non-standard native_thread type that you'd use instead of std::thread). That wouldn't be interchangeable with std::thread though, so I can see why you want to extend the existing type.

I was going to mention that p2019 suggests a very different approach for platform-specific properties of the new thread, but I see you already covered that.

I agree with Olivier that this would have to be conditionally supported, but that's already implied by the use of native_handle_type (which isn't required to exist). For libstdc++, we don't currently do anything special that's specific to C++ when creating a new thread, so we could support this exactly the same way.

However, I'm concerned that this changes the result of things like is_constructible<thread, int> or is_constructible<thread, void*>. On GNU/Linux thread::native_handle_type is just unsigned long, on MinGW-w64 it's uintptr_t, so this would mean you can construct a std::thread with any integer. On other targets it's a void*. I'd be happier if the new constructor required a tag type to say "I know what I'm doing", e.g. thread(from_native_t, native_handle_type) so you'd say std::thread t(std::from_native, thr);. Or JF's suggestion of a static member function would also work: auto t = std::thread::from_native(thr);

Right, thanks for these thoughts -- I agree that adding a public constructor taking int is not ideal.

I'll close this since I don't think it's reasonable to add this extension until the Standard has it, or there's a paper, or at least something. Thanks for your comments.

Right, thanks for these thoughts -- I agree that adding a public constructor taking int is not ideal.

It occurred to me later that a platform could in theory use void(*)() as pthread_t, as an opaque type that doesn't convert to other pointers or to integers. And then you'd have an ambiguity between the constructor taking a callable and the constructor taking a native_handle_type. Maybe unlikely, but not inconceivable.