proposal for pthreads extensions for suspend/resume

[andrewbachmann]

I'd like to propose the following extensions to the pthreads api:

pthread_suspend(pthread_t thread)
// suspend the thread, similar to solaris thr_suspend, or beos suspend_thread
pthread_continue(pthread_t thread)
// resume the thread, similar to solaris thr_continue, or beos resume_thread
pthread_attr_getstartsuspended(const pthread_attr_t *attr, bool*suspend)
pthread_attr_setstartsuspended(pthread_attr_t *attr,bool suspend)
// methods to control whether or not pthread_create starts the thread, or leaves it in suspended state

The application for this request is to simplify the OpenJDK port.

[kaliber]

Can you show which files from OpenJDK uses those functions? I cannot find it in openjdk6 and even openjdk7.

[andreasf]

Are these missing pthreads functions? Or is this a new proposal? If the latter, then I suggest to use a different prefix to avoid confusion.

[axeld]

We do not extend POSIX APIs. Use the native suspend_thread(), resume_thread(), and get_thread_info() functions instead - they will all work with pthreads as well.

[bonefish]

I'd be in favor of introducing a thread_id get_pthread_thread_id(pthread thread) at least, which would make it possible to use the Haiku thread API in the first place. A function for the reverse conversion would make sense as well. Though I'd suggest a name that implies that the thread then has to be handled with the pthread API (e.g. status_t convert_to_pthread(thread_id thread)), since otherwise the user would be leaking the pthread structures.

Regarding the pthread_attr_{get,set}startsuspended() suggestion, does something like this exists in other OSs?

[bonefish]

Since there were no objections, I've added get_pthread_thread_id(). convert_to_pthread() is more complicated to add, since the pthread object pointer has to be stored in the thread's TLS.

I wonder whether we shouldn't just always create pthread objects for Haiku threads, when they are created. The object would be leaked, when kill_thread() is invoked from another team (in all other cases we can do the cleanup), but I would consider this acceptable, since all other resources the thread still holds are leaked anyway. An obvious advantage besides the simplification in the pthread code is, that in libroot code we could use space in the pthread object as typed TLS instead of using actual TLS (e.g. in the syslog code).

[axeld]

Not sure if OS.h is the right place for that (as it's a system specific extension of the pthread API), but having a pthread object (which could even be named differently, then) for each thread sounds like a viable solution.

That won't simplify convert_to_pthread(), however. It would be possible to have a syscall that returns the TLS space of any thread, but due to race conditions, it might be better to have a syscall that reads only specific fields inside that TLS space.

[bonefish]

Replying to axeld:

Not sure if OS.h is the right place for that (as it's a system specific extension of the pthread API),

The options are either <OS.h>, <pthread.h>, or a new header. Since typedefs from both headers are needed (thread_id and pthread_id) <pthread.h> is not really a good choice, since we would pollute the POSIX namespace. A new header for just these two functions (or one ATM) seemed overkill, so <OS.h> was what I went for.

but having a pthread object (which could even be named differently, then) for each thread sounds like a viable solution.

That won't simplify convert_to_pthread(), however. It would be possible to have a syscall that returns the TLS space of any thread, but due to race conditions, it might be better to have a syscall that reads only specific fields inside that TLS space.

Nah, that's way to complicated. I was rather thinking of introducing a hash table or tree for the thread objects.

[axeld]

Replying to bonefish:

The options are either <OS.h>, <pthread.h>, or a new header. Since typedefs from both headers are needed (thread_id and pthread_id) <pthread.h> is not really a good choice, since we would pollute the POSIX namespace. A new header for just these two functions (or one ATM) seemed overkill, so <OS.h> was what I went for.

Okay, makes sense.

That won't simplify convert_to_pthread(), however. It would be possible to have a syscall that returns the TLS space of any thread, but due to race conditions, it might be better to have a syscall that reads only specific fields inside that TLS space.

Nah, that's way to complicated. I was rather thinking of introducing a hash table or tree for the thread objects.

Which would cause more overhead for a relatively rarely used feature (one that isn't even implemented as of now :-)). A "void* tls_get_for(thread_id thread, int32 slot)" would be easy to do, and would have a more generic usage as well.

[andrewbachmann]

I didn't see the replies to this issue until just now, for some reason I didn't get email notification.

In OpenJDK, in os_solaris.[hc]pp the methods thr_create and thr_continue are used to create and start the pthreads corresponding to java threads. thr_create is called with a flag to start suspended, which allows for reliable and straightforward java thread initiation.

In POSIX, flags such as THREAD_SUSPENDED (solaris) are provided via attributes. This was my rationale for proposing the pthread_attr_(get/set)startsuspended.

With the addition of the get_pthread_thread_id(), I am fine with not supplying pthread_suspend and pthread_continue. It does seem odd to require a pthread-oriented program to go through the thread_id in order to perform this functionality.

There are two barriers to creating the thread through spawn_thread. The first is that spawn_thread doesn't provide a means to specify the stack size. (functionality used by OpenJDK) The second is the lack of the proposed convert_to_pthread() method, which would be required to maintain the rest of the code using pthread methods.

I believe my proposal is the simplest and smallest change to provide the functionality used by OpenJDK.

[stippi]

Axel recently changed threads so every native Haiku thread is also a pthread now. Don't know if a "convert_to_pthread()" is still needed or not. But I thought that IDs are now interchangeable.

[bonefish]

Replying to andrewbachmann:

In OpenJDK, in os_solaris.[hc]pp the methods thr_create and thr_continue are used to create and start the pthreads corresponding to java threads. thr_create is called with a flag to start suspended, which allows for reliable and straightforward java thread initiation.

If this is only about thread initialization and it is for some reason not possible to do that initialization before actually creating the thread, you could simply use a wrapper function for the thread entry function. If, for some reason, the initialization has to happen in the thread creating the new thread, you could still use a standard locking primitive, like a mutex, to synchronize the two. Like in the creating thread:

• create mutex
• lock mutex
• create thread
• do initialization
• unlock mutex

And in the thread entry wrapper function:

• lock mutex
• destroy mutex
• call actual thread function

With the addition of the get_pthread_thread_id(), I am fine with not supplying pthread_suspend and pthread_continue. It does seem odd to require a pthread-oriented program to go through the thread_id in order to perform this functionality.

You actually don't even have to, since you could just use pthread_kill() with SIGSTOP and SIGCONT. I don't quite see, how that would already help with your initialization issue -- you'll have a huge race condition between creating the thread and suspending it. Unless you use something like I proposed above, but then I wouldn't see the need for suspending anymore.

I believe my proposal is the simplest and smallest change to provide the functionality used by OpenJDK.

Yeah, I agree with Axel, though, that we usually don't just add stuff to a standard (that's one of the things that makes standards suck). There are exceptions like when something is already used by several OSs and is thus a quasi-standard extension.

Replying to stippi:

Axel recently changed threads so every native Haiku thread is also a pthread now. Don't know if a "convert_to_pthread()" is still needed or not. But I thought that IDs are now interchangeable.

Pthreads don't have IDs, they are represented by a pthread object. So you'd still need to get the object pointer by thread_id, which is what the function would mainly do. It should also alter the detached state of the pthread. Normally (i.e. unless auto-detaching has specifically been requested) the pthread object lives on after the thread died -- it will be destroyed after a pthread_join() (the equivalent to a wait_for_thread()). Standard Haiku threads are auto-detached, so it would not be safe for other threads to play with their pthread object.