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

Don't round stack size up for created threads in Windows #94618

Merged
merged 1 commit into from
Mar 5, 2022

Conversation

lewisclark
Copy link
Contributor

@lewisclark lewisclark commented Mar 4, 2022

Fixes #94454

Windows does the rounding itself, so there isn't a need to explicity do the rounding beforehand, as mentioned by @ChrisDenton in #94454

The operating system rounds up the specified size to the nearest multiple of the system's allocation granularity (typically 64 KB). To retrieve the allocation granularity of the current system, use the GetSystemInfo function.

https://docs.microsoft.com/en-us/windows/win32/procthread/thread-stack-size

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @yaahc (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 4, 2022
@yaahc
Copy link
Member

yaahc commented Mar 4, 2022

LGTM, I wanna get a triple check from the windows maintainers before we approve but I'm pretty sure this is ready to go. Thank you very much!

@rustbot ping windows

@rustbot rustbot added the O-windows Operating system: Windows label Mar 4, 2022
@rustbot
Copy link
Collaborator

rustbot commented Mar 4, 2022

Hey Windows Group! This bug has been identified as a good "Windows candidate".
In case it's useful, here are some instructions for tackling these sorts of
bugs. Maybe take a look?
Thanks! <3

cc @arlosi @danielframpton @gdr-at-ms @kennykerr @luqmana @lzybkr @nico-abram @retep998 @rylev @sivadeilra @wesleywiser

@kennykerr
Copy link
Contributor

Looks good!

@yaahc
Copy link
Member

yaahc commented Mar 4, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Mar 4, 2022

📌 Commit 6843dd5 has been approved by yaahc

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 4, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 4, 2022
…ng, r=yaahc

Don't round stack size up for created threads in Windows

Fixes rust-lang#94454

Windows does the rounding itself, so there isn't a need to explicity do the rounding beforehand, as mentioned by `@ChrisDenton` in rust-lang#94454

> The operating system rounds up the specified size to the nearest multiple of the system's allocation granularity (typically 64 KB). To retrieve the allocation granularity of the current system, use the [GetSystemInfo](https://docs.microsoft.com/en-us/windows/win32/api/sysinfoapi/nf-sysinfoapi-getsysteminfo) function.

https://docs.microsoft.com/en-us/windows/win32/procthread/thread-stack-size
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 4, 2022
…ng, r=yaahc

Don't round stack size up for created threads in Windows

Fixes rust-lang#94454

Windows does the rounding itself, so there isn't a need to explicity do the rounding beforehand, as mentioned by ``@ChrisDenton`` in rust-lang#94454

> The operating system rounds up the specified size to the nearest multiple of the system's allocation granularity (typically 64 KB). To retrieve the allocation granularity of the current system, use the [GetSystemInfo](https://docs.microsoft.com/en-us/windows/win32/api/sysinfoapi/nf-sysinfoapi-getsysteminfo) function.

https://docs.microsoft.com/en-us/windows/win32/procthread/thread-stack-size
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 5, 2022
Rollup of 5 pull requests

Successful merges:

 - rust-lang#94362 (Add well known values to `--check-cfg` implementation)
 - rust-lang#94577 (only disable SIMD for doctests in Miri (not for the stdlib build itself))
 - rust-lang#94595 (Fix invalid `unresolved imports` errors for a single-segment import)
 - rust-lang#94596 (Delay bug in expr adjustment when check_expr is called multiple times)
 - rust-lang#94618 (Don't round stack size up for created threads in Windows)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 629e7aa into rust-lang:master Mar 5, 2022
@rustbot rustbot added this to the 1.61.0 milestone Mar 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-windows Operating system: Windows S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rare case of not properly rounding up thread stack size on Windows
6 participants