[patch] please implement pthread_attr_getstack()

[mjw]

I have created a patch to implement pthread_attr_getstack(). It also implements pthread_getattr_np() as a supporting function.

[bonefish]

pthread_getattr_np() is a non-standard GNU extension. I'm unsure whether we should provide it. Besides I wouldn't implement it by using the cached pthread creation attributes, but rather by collecting the info at that time. Most attributes we don't really support anyway -- so those can get the default values -- the stack address and size can be retrieved via get_thread_info().

[mjw]

Not quite an absolute "no", then.

Would it be more acceptable if I were to re-work the patch, removing pthread_getattr_np() and not caching the pthread_attr in pthread_create()?

So, the following would be valid: pthread_attr_init(attr) pthread_create(..., &attr, ...) pthread_attr_getstack(&attr)

[bonefish]

Replying to mjw:

Not quite an absolute "no", then.

Would it be more acceptable if I were to re-work the patch, removing pthread_getattr_np() and not caching the pthread_attr in pthread_create()?

Sure. Even better, if you also implemented the other stack-related attribute functions.

So, the following would be valid: pthread_attr_init(attr) pthread_create(..., &attr, ...) pthread_attr_getstack(&attr)

It would be valid, but I'm not sure that it would do what you expect. That is if you expect, that the pthread_attr_getstack() returns the stack address of the newly created thread. pthread_create() doesn't modify the supplied attributes -- it must not do that, since they are "const" and can't do that anyway, since that would prevent the attributes to be used for creating multiple threads. So, unless one explicitly sets the stack address, pthread_attr_getstack() should return NULL or some other placeholder.

Is there a special use case/ported software where the stack address and/or size is needed?

[mjw]

Yes, you are right. I was working to old docs. It looks as though pthread_attr_getstack() should return either NULL for the address base, or the address you passed into pthread_attr_setstack(). The only way to get a thread's stack base out of it is to call pthread_getattr_np() to get the actual thread's attributes before calling pthread_attr_getstack().

I was porting Kaffe Java VM which uses pthread_attr_getstack() in its unix pthreads code. I believe mono needs to know the thread stack base as well.

With that in mind, get_thread_info() looks like the correct thing to use, so this can be closed as invalid if you like.

[bonefish]

Replying to mjw:

Yes, you are right. I was working to old docs. It looks as though pthread_attr_getstack() should return either NULL for the address base, or the address you passed into pthread_attr_setstack(). The only way to get a thread's stack base out of it is to call pthread_getattr_np() to get the actual thread's attributes before calling pthread_attr_getstack().

I was porting Kaffe Java VM which uses pthread_attr_getstack() in its unix pthreads code. I believe mono needs to know the thread stack base as well.

Don't misunderstand me. I'm generally not opposed to adding non-POSIX functionality. It just seems pthread_getattr_np() is really GNU-only and not supported by other platforms. How is this solved for other non-GNU platforms (e.g. the BSDs)? Maybe there's another, more portable feature we could provide.

With that in mind, get_thread_info() looks like the correct thing to use, so this can be closed as invalid if you like.

The summary is valid at any rate. We should implement the missing pthread attribute functions earlier or later. :-)

[mjw]

I had an investigate and found that the BSDs implement their own function called pthread_attr_get_np().

NetBSD code is here: ​http://cvsweb.netbsd.org/bsdweb.cgi/~checkout~/src/lib/libpthread/pthread_attr.c?rev=1.12&content-type=text/plain&only_with_tag=MAIN

FreeBSD core is here: ​http://svn.freebsd.org/viewvc/base/head/lib/libthr/thread/thr_attr.c?revision=178446&view=markup

Man page is here: ​http://www.gsp.com/cgi-bin/man.cgi?section=3&topic=pthread_attr_get_np

This is almost identical to the GNU version, except that you must call pthread_attr_init() before calling these functions.

Looking at the complexity of the mono code, I don't think there is a nice cross platform way of doing this.

If you do: pthread_attr_init(attr); pthread_create(..., attr, ...); pthread_attr_getstack(attr, ...); then Linux, OpenSolaris and FreeBSD (and, I assume, other BSDs) return default values for stack base and stack size.

So, implementing pthread_attr_getstack() so that it returns the default values is simple. The only question is whether it is worth implementing either pthread_getattr_np() or pthread_attr_get_np(). If so, the argument comes down to whether you prefer BSD or GNU.

[bonefish]

I'd go with the GNU function for the time being. We can still decide to add the BSD function to our BSD compatibility library later.

[stippi]

I've read the comments, but it is unclear whether the patch would be ready as is.

[phoudoin]

Dunno myself.

Anyway, I may be wrong but I think the current patch's pthread_getattr_np() implementation will leak memory when called with a dst parameter already initialized by pthread_attr_init(), as recommended, like in this example:

size_t
get_thread_stack_size(pthread_t pid)
{
    pthread_attr_t attr;
    size_t size;

    pthread_attr_init(&attr);
    pthread_attr_get_np(pid, &attr);
    pthread_attr_getstacksize(&attr, &size); 
    pthread_attr_destroy(&attr);

    return(size);
}

Otherwise, Ingo raised concern about caching thread stack base address in thread internal attributes when such info can already be retrieved with get_thread_info(), wich will avoid to extent private pthread_attr struct.

[mmadia]

Replying to bonefish:

Replying to mjw:

With that in mind, get_thread_info() looks like the correct thing to use, so this can be closed as invalid if you like.

The summary is valid at any rate. We should implement the missing pthread attribute functions earlier or later. :-)

Setting the patch as obsolete, the above exchange seems to indicate that the patch was not acceptable as-is.

[kallisti5]

The lack of these pthread functions are breaking Ruby 2.x.x on Haiku.

See ​https://bugs.ruby-lang.org/issues/10811 for additional info.

[waddlesplash]

Reverting earlier milestone change

[waddlesplash]

Done in hrev52972 and hrev52973.

[nielx]

Assign tickets with status=closed and resolution=fixed within the R1/beta2 development window to the R1/beta2 Milestone