Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mono_thread_platform_create_thread crashing with pthread_attr_setstacksize invalid argument error #105490

Open
stephentoub opened this issue Jul 25, 2024 · 6 comments

Comments

@stephentoub
Copy link
Member

stephentoub commented Jul 25, 2024

Mono does not round up the argument to pthread_attr_setstacksize to the platform virtual page size which results in an EINVAL which will cause new Thread(ThreadStart, Int32) to fail


#105445 (comment)

The osx-x64 Debug Mono_MiniJIT_LibrariesTests leg crashed with:

mono_thread_platform_create_thread: pthread_attr_setstacksize failed, error: "Invalid argument" (22)

when trying to set a maxStackSize on a new thread to 100_000.

@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jul 25, 2024
@stephentoub stephentoub added this to the Future milestone Jul 25, 2024
@stephentoub stephentoub added the disabled-test The test is disabled in source code against the issue label Jul 25, 2024
@dotnet-policy-service dotnet-policy-service bot removed the untriaged New issue has not been triaged by the area owner label Jul 25, 2024
@lambdageek
Copy link
Member

I think this is because we're not aligning the requested stakc size to a multiple of the page size

compare Mono

if (stack_size)
set_stack_size = *stack_size;
else
set_stack_size = 0;
#ifdef HAVE_PTHREAD_ATTR_SETSTACKSIZE
if (set_stack_size == 0) {
#if HAVE_VALGRIND_MEMCHECK_H
if (RUNNING_ON_VALGRIND)
set_stack_size = 1 << 20;
else
set_stack_size = (SIZEOF_VOID_P / 4) * 1024 * 1024;
#else
set_stack_size = (SIZEOF_VOID_P / 4) * 1024 * 1024;
#endif
}
#ifdef PTHREAD_STACK_MIN
if (set_stack_size < PTHREAD_STACK_MIN)
set_stack_size = PTHREAD_STACK_MIN;
#endif

to coreCLR PAL

alignedStackSize = dwStackSize;
if (alignedStackSize != 0)
{
// Some systems require the stack size to be aligned to the page size
if (sizeof(alignedStackSize) <= sizeof(dwStackSize) && alignedStackSize + (GetVirtualPageSize() - 1) < alignedStackSize)
{
// When coming here from the public API surface, the incoming value is originally a nonnegative signed int32, so
// this shouldn't happen
ASSERT(
"Couldn't align the requested stack size (%zu) to the page size because the stack size was too large\n",
alignedStackSize);
palError = ERROR_INVALID_PARAMETER;
goto EXIT;
}
alignedStackSize = ALIGN_UP(alignedStackSize, GetVirtualPageSize());
}

@lambdageek lambdageek modified the milestones: Future, 9.0.0 Jul 25, 2024
@lambdageek
Copy link
Member

/cc @steveisok

@EgorBo
Copy link
Member

EgorBo commented Jul 25, 2024

@lambdageek I am not sure if that is related, but we also bumped default thread stack size on macOS for CoreCLR, from 512kb (which is fairly low) to 1 or 2 mb.

Since mac has 8mb for mainthread and 512kb for all secondary threads by default (on system level)

@lambdageek
Copy link
Member

limits.h in the MacOSX.sdk has

#if defined(__arm__) || defined(__arm64__)
#define PTHREAD_STACK_MIN 		16384
#else
#define PTHREAD_STACK_MIN 		8192
#endif

So requesting something smaller than the default ought to work. Mono does have a check that we ask for at least PTHREAD_STACK_MIN even if the user specified something smaller.

@lambdageek
Copy link
Member

Not a recent regression, btw. The following fails on mono going back to net6.0 at least (and probably older)

using System.Threading;

public class Program
{
    public static void Main()
    {
        var t = new Thread(() => {
            Console.WriteLine ("from a new thread");
            }, 16384 + 1);
        t.Start();
        t.Join();
    }
}

@lambdageek lambdageek removed the disabled-test The test is disabled in source code against the issue label Jul 25, 2024
@lambdageek
Copy link
Member

DeepEquals_TooDeepJsonDocument_ThrowsInsufficientExecutionStackException was updated to use a multiple of 64k for the stack size. So there's no disabled test anymore; it's just about aligning behavior now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants