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

Bug: Allocation in GC warning suppression proc blocks process #11728

Closed
jzakiya opened this issue Jan 8, 2022 · 8 comments · Fixed by #11729
Closed

Bug: Allocation in GC warning suppression proc blocks process #11728

jzakiya opened this issue Jan 8, 2022 · 8 comments · Fixed by #11729
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. kind:regression Something that used to correctly work but no longer works topic:stdlib:runtime
Milestone

Comments

@jzakiya
Copy link

jzakiya commented Jan 8, 2022

System: System76 laptop; i7-67000HQ, 4C|8T; 2.6-3.5 GHz; 16GB mem; Linux 5.15.13.

Running some code with 1.3.0 I found a bug in multi-threading that ran fine with 1.2.2.

Here's the code: https://gist.github.com/jzakiya/2b65b609f091dcbb6f792f16c63a8ac4

Here's how its compiled: crystal build twinprimes_ssoz.cr -Dpreview_mt --release

Here are some inputs that show working and non-working examples.

This input works: CRYSTAL_WORKERS=8 ./twinprimes_ssoz 50000000000000000 50000000019000000

This input hangs: CRYSTAL_WORKERS=8 ./twinprimes_ssoz 50000000000000000 50000000119000000

@jzakiya jzakiya added the kind:bug A bug in the code. Does not apply to documentation, specs, etc. label Jan 8, 2022
@jzakiya
Copy link
Author

jzakiya commented Jan 8, 2022

I see it runs when setting CRYSTAL_WORKERS to 1 or 2 but hangs for higher values.

@straight-shoota
Copy link
Member

I can confirm this on my machine. The example where the input hangs issues a GC warning on 1.2 but that's been disabled in 1.3 (#11289). It also really saturates all cores for a while on 1.2. But on 1.3 there's a short initial burst, then the threads seem to sit idle. So something's blocking.

It's interesting that it works for CRYSTAL_WORKERS=2.
For CRYSTAL_WORKERS=3 I get one more line of output (1 of 15 twinpairs done) before the program stalls.

@straight-shoota
Copy link
Member

Actually #11289 seems to be the culprit. Reverting that change makes the program run successfully on 1.3.0.

I think the reason is this:

format_string = String.new(msg)

The call to String.new allocates a new string. I'm pretty sure that's not a good idea inside an allocation warning hook. 🤦‍♂️

@straight-shoota straight-shoota added kind:regression Something that used to correctly work but no longer works topic:stdlib:runtime and removed topic:multithreading labels Jan 8, 2022
@straight-shoota straight-shoota changed the title Multi-threading hangs in 1.3.0 Bug: Allocation in GC warning suppression proc blocks process Jan 8, 2022
@jzakiya
Copy link
Author

jzakiya commented Jan 8, 2022

Thanks @straight-shoota for quickly tracking this down and fixing.

Will there be a 1.3.1 to include this, et al, fixes before 1.4.0?

@straight-shoota
Copy link
Member

Yes, this regression should land in 1.3.1.

@straight-shoota straight-shoota added this to the 1.3.1 milestone Jan 8, 2022
@jzakiya
Copy link
Author

jzakiya commented Jan 8, 2022

Thank you!

@yxhuvud
Copy link
Contributor

yxhuvud commented Jan 9, 2022

🤦‍♂️ Argh. I should have seen that was an issue.

@straight-shoota
Copy link
Member

Yeah we all missed that despite C bindings needing extra care (especially without specs) 🙈

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. kind:regression Something that used to correctly work but no longer works topic:stdlib:runtime
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants