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

x/playground: exec.CommandContext(...).Run() does not honor context timeout #38855

Closed
hlin117 opened this issue May 4, 2020 · 8 comments
Closed
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@hlin117
Copy link

hlin117 commented May 4, 2020

What version of Go are you using (go version)?

1.14.2, as stated on the Go Playground (as of May 4th, 2020).

Does this issue reproduce with the latest release?

Yes, 1.14.2 is the latest release.

What operating system and processor architecture are you using (go env)?

Environment is the Go Playground.

Note that I cannot reproduce this error on my 2019 Macbook, running 1.14.2.

What did you do?

I ran the demo on the Go documentation with os/exec:CommandContext, I discovered that the timeout example seemed to have strange behavior.

Screen Shot 2020-05-04 at 11 09 33 AM

It seemed the demo was hitting the timeout limit of the Go Playground, ignoring the 100ms context timeout.

Modifying the demo a little bit, I discovered that the timeout on the Go Playground was being ignored completely. See below screenshot.

Screen Shot 2020-05-04 at 11 13 06 AM

See Go Playground link: https://play.golang.org/p/ckZbgZ7n1tM

What did you expect to see?

The 100ms timeout from the context should have been honored when running exec.CommandContext(...). It seems based on #21880 and me running the demo on my laptop that this behavior worked in previous releases, and this bug might exist on different environments.

What did you see instead?

The 100ms timeout is being ignored on the Go Playground environment.

@ianlancetaylor
Copy link
Contributor

Does the playground really let you exec other programs?

@hlin117
Copy link
Author

hlin117 commented May 4, 2020

I'm assuming from the documentation (whose code hasn't changed for several releases) that this has been the case for several years (based on #21880, since 2017).

@ianlancetaylor
Copy link
Contributor

issue #21880 is not about the Go playground. I read your issue as saying that your program works correctly on your local system, but fails on the Go playground. Is that right? Thanks.

@hlin117
Copy link
Author

hlin117 commented May 4, 2020

Yes that's correct. It works on my local system, but not on the Go playground.

@ianlancetaylor
Copy link
Contributor

Thanks. I am saying: does using the os/exec package ever work on the Go playground at all? The Go playground is a very limited execution environment. It doesn't support running arbitrary programs.

@prattmic
Copy link
Member

prattmic commented May 4, 2020

cc @toothrot

The playground is now running on gVisor, so it will allow executing other programs unless the Go build itself prevents calling execve [1]. I could be wrong, but I thought faketime was the only change from a standard build now.

I'd imagine it is faketime that would break timeouts.

[1] That said, I wouldn't necessarily expect other binaries, like sleep, to exist in the filesystem.

@toothrot
Copy link
Contributor

toothrot commented May 4, 2020

@prattmic That is correct, as of the changes in #25224. The container is based on busybox:glibc, so there are some built in programs.

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 5, 2020
@dmitshur dmitshur changed the title os/exec: CommandContext(...).Run() does not honor context timeout on Go Playground x/playground: exec.CommandContext(...).Run() does not honor context timeout May 5, 2020
@gopherbot gopherbot added this to the Unreleased milestone May 5, 2020
@toothrot
Copy link
Contributor

I believe that this issue is related to faketime. I can't think of any easy way to resolve that.

In order to test timeouts with faketime, you'll have to add code that advances the fake clock: https://play.golang.org/p/9ERCtbudekx

I'm going to close this as it seems to be working as intended. Also, I believe we're generally fine with the ability to exec inside the gvisor'd sandbox docker container. Please let me know if I am mistaken.

@golang golang locked and limited conversation to collaborators May 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

6 participants