Skip to content
New issue

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

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

Already on GitHub? # to your account

Replace test Task with Thread with increased stack size #1883

Merged
merged 1 commit into from
Jan 26, 2025

Conversation

BCSharp
Copy link
Member

@BCSharp BCSharp commented Jan 26, 2025

Increases the stack size used by the thread running tests to 2 MiB. Reportedly the stack size of background treads on .NET 8.0 on macOS is extremely small (0.5 MiB), so it is easy to hit the limit.

This is essentially the last commit from #1879, which does not belong there but I pushed it there to test it with CI. I could not reproduce the stack overflow locally, probably because I develop on ARM which extensively uses registers for parameter passing so its stack usage is lower than on x64. Once this PR is merged, I will rebase #1879 on top of the current main branch (sans the last commit) and presumably the tests will pass.

Copy link
Contributor

@slozier slozier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I guess this has the added advantage that it'll try to abort on .NET Framework?

@BCSharp
Copy link
Member Author

BCSharp commented Jan 26, 2025

I guess this has the added advantage that it'll try to abort on .NET Framework?

I suppose so. Although we had misconfigured long tasks in the past that were timing out prematurely, my understanding is that the timeout is here to prevent the whole test run to hang indefinitely because of one misbehaving test. If so, the thread will never complete and basically hang on to its resources until the whole process terminates. ThreadAbort may leak resources, but it is still better than not releasing them at all. At least we get the stack space back.

The whole solution is based on #411 courtesy of @slide.

@BCSharp BCSharp merged commit 49e18cb into IronLanguages:main Jan 26, 2025
8 checks passed
@BCSharp BCSharp deleted the stack_increase branch January 26, 2025 19:39
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants