Skip to content

Add Fiber::Stack#size#16420

Merged
straight-shoota merged 1 commit intocrystal-lang:masterfrom
ysbaddaden:refactor/add-fiber-stack-size
Nov 26, 2025
Merged

Add Fiber::Stack#size#16420
straight-shoota merged 1 commit intocrystal-lang:masterfrom
ysbaddaden:refactor/add-fiber-stack-size

Conversation

@ysbaddaden
Copy link
Collaborator

A tiny refactor to save the stack size in the Fiber::Stack object, so we can free stacks without assuming the stack's actual size.

Extracted from #15885 where the thread pool will have to free stacks outside of execution contexts and thus outside of any stack pool, for example.

Comment on lines +11 to +13
# FIXME: sometimes gc/boehm reports weird stack limits on linux (over
# 2GB) at least, so we always cast to i32 without overflow checks.
@size = (@bottom - @pointer).to_i32!
Copy link
Member

Choose a reason for hiding this comment

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

thought: 2GB stack size is probably wrong. But would it make sense to use a different type for size? UInt32 for example?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't recall exactly but I think the comment says "over 2GB" to match the i32 limit. It would likely fail just the same with a u32 limit.

Either @pointer and/or @bottom felt like it was a random value. Weirdly enough, only the reported stack pointers are invalid, and the GC still works correctly, which is 🤨

@straight-shoota straight-shoota added this to the 1.19.0 milestone Nov 25, 2025
@straight-shoota straight-shoota moved this from Review to Approved in Multi-threading Nov 25, 2025
@straight-shoota straight-shoota merged commit 4e1fdc8 into crystal-lang:master Nov 26, 2025
45 of 46 checks passed
@github-project-automation github-project-automation bot moved this from Approved to Done in Multi-threading Nov 26, 2025
@ysbaddaden ysbaddaden deleted the refactor/add-fiber-stack-size branch November 27, 2025 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants