On Sat, Aug 12, 2017 at 12:38:53AM -0500, Bobby Bingham wrote:
> I've got a glibc-linked binary I'm trying to get running, and I'm
> running into the usual stack overflow for threads other than the main
> thread because of musl's smaller default stack size.  So I decided to
> see if I could LD_PRELOAD a library with a constructor that used
> pthread_setattr_default_np to increase the default stack size.
> 
> It turns out that it doesn't work for this binary because it passes a
> thread attribute object to pthread_create without explicitly setting the
> stack size in the attribute object.  According to the man page, the
> values set with pthread_getattr_default_np are only used if no attribute
> object is passed to pthread_create, which is how musl behaves.
> 
> It turns out this isn't what glibc does for stack size (but is for guard
> size).
> 
> glibc's pthread_attr_init sets the stack size field in the attribute
> object to zero, which is interpreted to mean the default stack size at
> whatever time it's queried.  The attached program will print two
> different stack sizes on glibc.
> 
> I think that the glibc behavior is problematic because
> pthread_attr_getstacksize can return a value different than what will
> actually be used if the default is changed before pthread_create is
> called.
> 
> However, if we changed pthread_attr_init to initialize the stack size
> based on the current default at the time its called, we'd be a little
> closer to glibc's behavior, but without the quirk I mentioned above.
> And it would fix my use case :)
> 
> Thoughts?

Somehow I missed this at the time, or forgot about it, and just
rediscovered the same issue. Actually I thought glibc ignored the
default set by pthread_setattr_default_np in this case, because I
wasn't aware of the (likely non-conforming, unless
pthread_attr_getstacksize patches it up) way zero is used in glibc,
and I was about to propose we diverge from glibc here just because the
current behavior is not useful for the intended purpose.

The whole reason pthread_setattr_default_np was added was to allow
easy workarounds for applications/libraries that assume large stacks:
so packagers can just add an early call to pthread_setattr_default_np
to ensure that all threads created have sufficiently large stacks. But
some code, notably SDL, creates an attribute object to set other
properties, leaving the stack size alone, and thereby gets the
musl-default stack size rather than the new default configured by
pthread_setattr_default_np.

The attached patch fixes this, and I think I'll apply it, but I'm not
entirely happy with the situation before or after this change, since
pthread_setattr_default_np is thread-unsafe and thus it's unsafe to
call from ctors in libraries that might be dynamic-loaded or where
threads might have already been created by other libraries' ctors.

I think we should probably have pthread_setattr_default_np use a_cas
to update the defaults, changing them from size_t to volatile int
(there's no need to support defaults >INT_MAX; doing so would just be
harmful) so a_cas is usable on them. Then the consumers,
pthread_create and pthread_attr_init, would be getting relaxed-order
atomics rather than data races. Assuming there's something else to
order the pthread_setattr_default_np with respect to pthread_create or
pthread_attr_init (e.g. being called in the same thread) you know the
latter will get the latest-set default; otherwise it gets *some valid*
default from the past or present.

Rich