Skip to content

Fixed: Fix SIGKILL, am socket result frame and socket fd bugs in termux-shared - #5246

Open
BuildWithAni wants to merge 1 commit into
termux:masterfrom
BuildWithAni:fix/termux-shared-execution-fixes
Open

Fixed: Fix SIGKILL, am socket result frame and socket fd bugs in termux-shared#5246
BuildWithAni wants to merge 1 commit into
termux:masterfrom
BuildWithAni:fix/termux-shared-execution-fixes

Conversation

@BuildWithAni

Copy link
Copy Markdown

Fixes the following bugs found while reviewing the codebase. All changes are in the termux-shared module.

  1. AppShell.killIfExecuting() never kills the process (AppShell.java). The condition if (isExecuting()) is always false after setStateFailed() is called in AppShell.execute(), so the kill() call was dead code. Killing is now done unconditionally. This also affects PhantomProcessUtils and other APP_SHELL users where a shell process that failed to start (e.g. killed with signal 9 by android phantom process trimming) would never be killed on timeout.

  2. AmSocketServer.processAmClient() missing return (AmSocketServer.java). After sending the RESULT_ERROR_FRAME for a failed command, execution continued and sent a second RESULT_SUCCESS_FRAME, causing the caller to wait forever for a result.

  3. LocalClientSocket.available() checks the wrong fd (LocalClientSocket.java). It polled the server socket's fd instead of the client's own mFD, so the read timeout logic never triggered and reads could hang forever.

All modules compile successfully:

BUILD SUCCESSFUL

…ux-shared

- Fixed: Send SIGKILL to process in AppShell.killIfExecuting() unconditionally since setStateFailed() sets state to FAILED and isExecuting() will always be false after that, so SIGKILL was never sent
- Fixed: Return early in AmSocketServer.processAmClient() after sending error result to client so that a bogus success result frame is not sent afterwards
- Fixed: Use client socket fd instead of server socket fd in LocalClientSocket.available() to check for available data
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.

1 participant