Hello @abdollar,

This feels ugly! This kind of thing should not be set by libraries. But I agree that (like that horrible stdio hack for Windows) it makes pragmatic sense.

How about checking the current stacksize and making sure we don't lower it? There would then be less danger of fighting with some other library over stacksize. The env var could perhaps be renamed VIPS_MIN_STACK_SIZE.

I would use guint64 vips__parse_size( const char *size_string ) instead of atoi(). It'll allow strings like "4m".

I agree, 2m seems a sensible minimum.

@jcupitt - updated changes and description per your suggestions of VIPS_MIN_STACK_SIZE. I added a warning if the current stack size is larger indicating we are not going to change it and included the current settings. So - I was debating adding this and going back and forth on this change. It seems to make sense due to the init section and thread pool being managed here and it feels right especially for those of us with containers and environment variables. Thanks.

Does the stack size need to be aligned to the page size?

https://linux.die.net/man/3/pthread_attr_setstacksize
https://linux.die.net/man/2/getpagesize

I found this issue discussing the 80kb limit and how to step around it. There are interesting links to the solutions other projects have cooked up.

https://github.com/voidlinux/void-packages/issues/4147

Our case is slightly different from the large stack frames allocations they discuss. Poppler walks the document structure recursively, so the low limit simply makes some common user input impossible to handle.

@lovell I think you need to be page-aligned for setstack, but the stack size does not need to be.

It looks like not all pthread implementations have pthread_setattr_default_np() -- musl added this in 2016, for example. We probably need to add a test to configure. @abdollar are you OK adding a configure test? I can do this if you'd rather not.

@jcupitt - I have updated configure and added a library check to include HAVE_PTHREAD_DEFAULT_NP using AC_CHECK_LIB([pthread]. I removed the linux check, but could add it back, though this seems to make more sense for any thing with pthreads and pthread_setattr_default_np.

Nice! Looks good to me, anyone have any other comments?

OK, let's merge! Thank you for working on this, @abdollar, it's a nice improvement.

I added _GNU_SOURCE to get it to work on glibc. I added a note to the docs, stopped the message if the stack is already large enough, and did a minor reformat to libvips conventions.