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

Regression: Interrupted system call in CI #14350

Closed
straight-shoota opened this issue Mar 7, 2024 · 6 comments · Fixed by #14351
Closed

Regression: Interrupted system call in CI #14350

straight-shoota opened this issue Mar 7, 2024 · 6 comments · Fixed by #14351
Labels
kind:bug kind:regression Something that used to correctly work but no longer works

Comments

@straight-shoota
Copy link
Member

Looks like we have a regression in master. The build for current HEAD is broken:

https://github.com/crystal-lang/crystal/actions/runs/8178703060/job/22363225395

Error message:

Error writing file (#IO::FileDescriptor:0x7ff6dd163e00): Interrupted system call (IO::Error)

A very similar error had previously appeared in #14343 (failed job) but it went through after a retry, so I considered it a fluke.
It's entirely plausible that this PR is causing the error. It does some changes to the IO system interface.
But it could also be something else. We had some PRs that change concurrency lately, for example.

The master CI workflow has another interesting failure in the MT job: https://github.com/crystal-lang/crystal/actions/runs/8178703060/job/22363221791
It seems that the state memory of Colorize is compromised because it inserts reset codes (0;) even when the last state is default and thus wouldn't require it. Not sure if this is even connected to the interrupted syscall, but it would point towards a concurrency issue.

@straight-shoota straight-shoota added kind:bug kind:regression Something that used to correctly work but no longer works labels Mar 7, 2024
@ysbaddaden
Copy link
Contributor

@straight-shoota @HertzDevil the solaris PR changes the signal handler to use sigaction(2) instead of signal(2) and we get interrupted syscalls 🤔

https://github.com/crystal-lang/crystal/pull/14343/files#diff-defc0d953337c02ce30da590dbf3a9f0daa536c9fae6ad381e2af06ffab1eb60

@ysbaddaden
Copy link
Contributor

Reading the signal(7) man page, we might just need to add the SA_RESTART flag?

@ysbaddaden
Copy link
Contributor

From the signal(2) man page:

The situation on Linux is as follows:

  • The kernel's signal() system call provides System V semantics.
  • By default, in glibc 2 and later, the signal() wrapper function does not invoke the kernel system call. Instead, it calls sigaction(2) using flags that supply BSD semantics.

The BSD semantics are equivalent to calling sigaction(2) with the following flags:

sa.sa_flags = SA_RESTART;

@HertzDevil
Copy link
Contributor

HertzDevil commented Mar 7, 2024

It looks like none of the bindings define LibC::SA_RESTART, so maybe it's best to use sigaction there only when {{ flag?(:solaris) }} is true, unless someone could find the correct values on all supported systems?

@ysbaddaden
Copy link
Contributor

I pushed without checking 🤦

Luckily, I have a copy of many /usr/include on my computer from when I wrote the posix shard. They date from many years back, but constants for syscalls are unlikely to ever change.

That being said, I wish I could just say "I want SA_RESTART please figure its actual value".

@straight-shoota
Copy link
Member Author

but constants for syscalls are unlikely to ever change.

Yeah, that would lead to very surprising results 😆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug kind:regression Something that used to correctly work but no longer works
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants