Open
Conversation
Signed-off-by: Vedran Jukic <vedran.jukic@gmail.com> Made-with: Cursor
7c52e6e to
6545e4e
Compare
Signed-off-by: Toma Puljak <toma.puljak@hotmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes the
process.exectimeout not being respected. When a command likesleep 5; echo 'hello'is executed withtimeout=2, the timeout error is only thrown after the full 5 seconds instead of after 2 seconds.Root cause: The SDK wraps user commands as
sh -c "echo '<base64>' | base64 -d | sh", creating a process tree (outersh→ pipe → innersh→sleep 5). When the daemon's timeout fired,cmd.Process.Kill()only sent SIGKILL to the outershprocess. The grandchild processes inherited the pipe file descriptors and continued running, causingCombinedOutput()to block until they exited naturally.Fix (two parts):
Daemon (
apps/daemon/pkg/toolbox/process/execute.go): SetSetpgid: trueon the command so all child processes are placed in a new process group, then usesyscall.Kill(-pid, SIGKILL)to kill the entire group on timeout. This ensures all descendant processes are terminated immediately.SDK (
libs/sdk-python/src/daytona/_sync/process.py,libs/sdk-python/src/daytona/_async/process.py): Added_request_timeout=http_timeout(timeout)to theexecute_command()call inexec(), matching the pattern already used byexecute_session_command(). This prevents the HTTP client from blocking indefinitely if the daemon-side timeout fails to respond promptly.Documentation
Related Issue(s)
This PR addresses issue #2602