Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.

fix Issue 20497 - thread with limited stackspace crashes depending on…#2904

Merged
dlang-bot merged 1 commit intodlang:stablefrom
rainers:issue20497
Jan 11, 2020
Merged

fix Issue 20497 - thread with limited stackspace crashes depending on…#2904
dlang-bot merged 1 commit intodlang:stablefrom
rainers:issue20497

Conversation

@rainers
Copy link
Copy Markdown
Member

@rainers rainers commented Jan 10, 2020

@dlang-bot
Copy link
Copy Markdown
Contributor

Thanks for your pull request, @rainers!

Bugzilla references

Auto-close Bugzilla Severity Description
20497 critical thread with limited stackspace crashes depending on size of TLS

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "stable + druntime#2904"

@dlang-bot dlang-bot added the Bug Fix Include reference to corresponding bugzilla issue label Jan 10, 2020
@rainers
Copy link
Copy Markdown
Member Author

rainers commented Jan 10, 2020

The output of the test will hopefully show what platforms are actually using the stack for TLS.

@rainers rainers force-pushed the issue20497 branch 3 times, most recently from 23fa0e0 to dfbfcca Compare January 10, 2020 23:44
@rainers rainers requested a review from Burgos as a code owner January 10, 2020 23:44
Comment on lines +3188 to +3190
// TLS uses the top of the stack, so add its size to the requested size
sz += externDFunc!("rt.sections_elf_shared.sizeOfTLS",
size_t function() @nogc nothrow)();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Isn't it better to add version(CRuntime_Glibc) here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Probably, as FreeBsd doesn't seem to have this issue. I've added the version check.

@kubo39
Copy link
Copy Markdown
Contributor

kubo39 commented Jan 11, 2020

Nit: In glibc, we can use __pthread_get_minstack which is added in 2.15.

… size of TLS

on Posix systems, add the TLS size to the requested stack size for new threads
@rainers
Copy link
Copy Markdown
Member Author

rainers commented Jan 11, 2020

Nit: In glibc, we can use __pthread_get_minstack which is added in 2.15.

Indeed, looks like a simpler solution. When searching for it I noticed a couple of bug reports due to undefined references to this symbol. Would it raise the libc version requirements for druntime?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Bug Fix Include reference to corresponding bugzilla issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants