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

compiler_gym::util::LocalShellCommand::checkCall() does not kill process on timeout #677

Closed
ChrisCummins opened this issue May 10, 2022 · 0 comments · Fixed by #741
Closed
Assignees
Labels
Bug Something isn't working good first issue Good for newcomers

Comments

@ChrisCummins
Copy link
Contributor

🐛 Bug

If the process fails to terminate within the specified timeout period, it is abandoned:

if (!wait_for(process, timeout())) {
return Status(StatusCode::DEADLINE_EXCEEDED,
fmt::format("Command '{}' failed to complete within {} seconds", commandline(),
timeoutSeconds()));
}

Instead, the

To Reproduce

This unit test covers the timeout case:

TEST(Subprocess, checkCallTimeout) {
Command proto;
proto.add_argument("sleep");
proto.add_argument("10");
proto.set_timeout_seconds(1);
LocalShellCommand cmd(proto);
#ifdef __APPLE__
// NOTE(github.com/facebookresearch/CompilerGym/issues/399): This test will
// fail when the wait_for() problem is addressed.
EXPECT_TRUE(cmd.checkCall().ok());
#else // Linux
const auto status = cmd.checkCall();
EXPECT_EQ(status.error_code(), StatusCode::DEADLINE_EXCEEDED);
EXPECT_EQ(status.error_message(), "Command 'sleep 10' failed to complete within 1 seconds");
#endif
}

run it using bazel test //tests/util:SubprocessTest.

Expected behavior

The child process should be terminated.

Environment

Please fill in this checklist:

  • CompilerGym: 0.2.3
  • How you installed CompilerGym (pip, source): source
  • OS: Ubuntu 20.04
  • Python version: 3.8
  • Build command you used (if compiling from source): make install

Additional context

Reported by @uduse #326 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant