Skip to content

Conversation

@kolyshkin
Copy link
Contributor

Inspired by #4680. The idea here is hide the retry-on-EINTR functionality inside a package, as well as provide error wrappers.

A few more wrappers is to be added.

PS I'm not sure about the package name, perhaps sys or uunix is a better name. I want something short and simple.

@kolyshkin
Copy link
Contributor Author

The alternative to this is to create something like moby/sys/unix which will work the same as x/sys/unix, except for:

  • rich errors (with added context such as syscall name);
  • retrying on EINTR.

@kolyshkin kolyshkin requested a review from rata March 26, 2025 00:53
@kolyshkin
Copy link
Contributor Author

@rata WDYT? The motivation here is,

  • abstracting away all the hacks we need to do to make it work in a separate package is better than sprinkling the code with RetryOnEINTR;
  • we can kill two birds with one stone by also wrapping errors.

PS In an ideal world, I wish for a package like this ("wrapped unix") to already exist.

@rata
Copy link
Member

rata commented Mar 26, 2025

@kolyshkin Makes sense to me! This is definitely some direction I thought to explore after merging the other PR. When playing with our openat2 yesterday (to address your comment), I was thinking of thin wrappers that just do this but I wasn't sure it would be liked because of the existing openat wrapper that was doing more stuff.

But you went ahead and did both things at once. Perfect :)

Copy link
Member

@rata rata left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Comment on lines 11 to 12
return os.NewSyscallError("dup3", retryOnEINTR(func() error {
return unix.Dup3(oldfd, newfd, flags)
Copy link
Member

Choose a reason for hiding this comment

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

nit: I think splitting this in more lines can be clearer. The functio that takes several params, the third of which is another function that takes a function param that is defined inline is a little bit too much, IMHO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed this and also Sendmsg in the same way.

This package is to provide unix.* wrappers to ensure that:
 - they retry on EINTR;
 - a "rich" error is returned on failure.

 A first such wrapper, Sendmsg, is introduced.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Drop the libcontainer/system/exec, and use the linux.Exec instead.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@AkihiroSuda AkihiroSuda merged commit f3df262 into opencontainers:main Mar 28, 2025
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants