facebookresearch / CompilerGym

Reinforcement learning environments for compiler and program optimization tasks

https://compilergym.ai/

MIT License

906 stars 127 forks source link

Fix LocalShellCommand::checkCall() does not kill process on timeout. #741

Closed thecoblack closed 2 years ago

thecoblack commented 2 years ago

Fixes #677

codecov-commenter commented 2 years ago

Codecov Report

Merging #741 (b58e83b) into development (b58e83b) will not change coverage. The diff coverage is n/a.

:exclamation: Current head b58e83b differs from pull request most recent head 4d9fa97. Consider uploading reports for the commit 4d9fa97 to get more accurate results

@@             Coverage Diff              @@
##           development     #741   +/-   ##
============================================
  Coverage        88.75%   88.75%           
============================================
  Files              129      129           
  Lines             7855     7855           
============================================
  Hits              6972     6972           
  Misses             883      883

ChrisCummins commented 2 years ago

Hey @thecoblack, thanks for taking a look at this. Does terminate() have a timeout?

Also, there's a minor indentation issue to fix.

Cheers, Chris

thecoblack commented 2 years ago

Hi @ChrisCummins, terminate() does not provide a timeout. Here is the definition from official docs:

void terminate();
void terminate(std::error_code &) noexcept;

It kills the process through an implementation of SIGKILL on posix and TerminateProcess on windows when is called.

ChrisCummins commented 2 years ago

Thanks for the context, and for the bug fix 🙂 . LGTM, merging!

Cheers, Chris